Conversation
Walkthrough계약 채팅의 Redis 기반 입장/Presence 관리, WebSocket CONNECT 인증 인터셉터 추가, 최종계약 요청/수락 API 및 서비스 로직(법률 검증 포함) 도입, LegalityDTO·매퍼·리포지토리·컨트롤러 경로·시그니처 변경, debug 엔드포인트 및 chat URL에 homeId 추가. Changes
Sequence Diagram(s)sequenceDiagram
participant Owner as Landlord
participant API as ContractController
participant ChatSvc as ContractChatService
participant FixSvc as ContractFixService
participant Repo as ContractMongoRepository
Owner->>API: POST /specialContract/final-request
API->>ChatSvc: requestFinalContract(contractChatId, ownerId)
ChatSvc->>ChatSvc: Redis coordination / persist flag
ChatSvc-->>Owner: Ack
Buyer->>API: POST /specialContract/final-accept {isAccepted}
API->>ChatSvc: acceptFinalContract(contractChatId, buyerId, isAccepted)
alt accepted
ChatSvc->>Repo: saveSpecialContract / clearSpecialContracts
ChatSvc->>FixSvc: getLegality(contractChatId, buyerId)
FixSvc->>Repo: load contract data
FixSvc-->>ChatSvc: LegalityDTO
ChatSvc-->>Buyer: AiMessageLegal / 알림
else rejected
ChatSvc-->>Buyer: 거절 알림
end
sequenceDiagram
participant Client
participant WS as WebSocketConfig
Client->>WS: STOMP CONNECT (Authorization: Bearer JWT 또는 X-User-Id)
WS->>WS: 인터셉트, JWT payload 디코드 또는 헤더 사용해 Principal 생성
WS-->>Client: CONNECTED (Principal attached)
Client->>WS: SUBSCRIBE /topic..., /user/queue...
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java
Fixed
Show fixed
Hide fixed
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java
Fixed
Show fixed
Hide fixed
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java
Fixed
Show fixed
Hide fixed
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java
Fixed
Show fixed
Hide fixed
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java
Fixed
Show fixed
Hide fixed
src/main/java/org/scoula/domain/contract/service/ContractFixService.java
Fixed
Show fixed
Hide fixed
src/main/java/org/scoula/domain/contract/service/ContractFixService.java
Fixed
Show fixed
Hide fixed
| accessor.setUser(auth); | ||
| log.info( | ||
| "✅ WebSocket 인증 성공 (JWT) - User: {}", | ||
| userEmail); |
Check failure
Code scanning / CodeQL
Insertion of sensitive information into log files High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we should avoid logging the actual value of userEmail (the user's email address) in the log message. Instead, we can log a generic success message or, if necessary, log only non-sensitive information (such as a boolean indicating success). Specifically, in src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java, replace the log statement on line 107-108 so that it does not include the userEmail value. No additional imports or methods are required; simply update the log statement to remove the sensitive data.
| @@ -104,8 +104,7 @@ | ||
| Collections.emptyList()); | ||
| accessor.setUser(auth); | ||
| log.info( | ||
| "✅ WebSocket 인증 성공 (JWT) - User: {}", | ||
| userEmail); | ||
| "✅ WebSocket 인증 성공 (JWT) - 사용자 인증 성공"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (6)
src/main/java/org/scoula/domain/precontract/service/OwnerPreContractServiceImpl.java (4)
505-510: 민감정보(PII) 로그 노출 위험: 전체 Document 로깅 제거 필요
OwnerMongoDocument전체를 로깅하면 계좌번호 등 민감정보가 그대로 로그에 남습니다. 요약 정보(식별자 중심)만 남기도록 수정이 필요합니다.다음과 같이 수정하세요:
- log.info("변환된 document: {}", document); + // PII 노출 방지를 위해 전체 객체 대신 키 정보만 로깅 + log.info("Mongo 저장 시작 - contractChatId: {}, userId: {}", dto.getContractChatId(), dto.getUserId()); ... - log.info("Mongo 저장 결과: {}", result); + // 저장 완료 시에도 민감정보는 로깅하지 않음 + log.info("Mongo 저장 완료 - contractChatId: {}, userId: {}", result.getContractChatId(), result.getUserId());
576-581: 민감정보(PII) 포함 가능 JSON 전체 로깅 금지
aiRequest전체를 JSON으로 직렬화하여 로그에 남기는 것은 RRN/계좌/주소 등 다수 PII 노출 가능성이 큽니다. 요약 정보만 로깅하도록 변경해 주세요.- log.info("AI 요청 데이터: {}", objectMapper.writeValueAsString(aiRequest)); + // 민감정보 노출 방지를 위해 요약만 로깅 + log.info("AI 요청 요약 - contractChatId: {}, ownerPrecheckId: {}, specialTermsCount: {}", + aiRequest.getOwnerData().getContractChatId(), + aiRequest.getOwnerData().getOwnerPrecheckId(), + aiRequest.getOcrData() != null && aiRequest.getOcrData().getSpecialTerms() != null + ? aiRequest.getOcrData().getSpecialTerms().size() + : 0);
427-443: NPE 위험: contractChat/chatRoom null 체크 필요
findByContractChatId또는findByUserAndHome가 null을 반환할 경우 NPE가 발생합니다. 명시적으로 검증하고 비즈니스 예외를 던지세요.- ContractChat contractChat = contractChatMapper.findByContractChatId(contractChatId); - ChatRoom chatRoom = - chatRoomMapper.findByUserAndHome( - contractChat.getOwnerId(), - contractChat.getBuyerId(), - contractChat.getHomeId()); + ContractChat contractChat = contractChatMapper.findByContractChatId(contractChatId); + if (contractChat == null) { + throw new BusinessException(OwnerPreContractErrorCode.OWNER_SELECT, "계약 채팅 정보가 존재하지 않습니다."); + } + + ChatRoom chatRoom = + chatRoomMapper.findByUserAndHome( + contractChat.getOwnerId(), + contractChat.getBuyerId(), + contractChat.getHomeId()); + if (chatRoom == null) { + throw new BusinessException(OwnerPreContractErrorCode.OWNER_SELECT, "채팅방이 존재하지 않습니다."); + }
84-87: RuntimeException 대신 도메인 예외 사용검증 실패 시
RuntimeException은 모호하며 글로벌 에러 핸들러/응답 규격과도 어울리지 않습니다.BusinessException+ 전용 에러코드로 교체하세요.- if (!result) { - log.warn("신분증 진위 확인 실패"); - throw new RuntimeException("신분증 검증 실패"); - } + if (!result) { + log.warn("신분증 진위 확인 실패"); + throw new BusinessException(OwnerPreContractErrorCode.OWNER_ID_VERIFICATION_FAILED, "신분증 검증 실패"); + }추가로
OwnerPreContractErrorCode에 전용 코드를 정의해 주세요:// OwnerPreContractErrorCode.java OWNER_ID_VERIFICATION_FAILED, // 신분증 진위 확인 실패필요하시다면 관련 enum/코드 추가 PR 스캐폴딩을 제공해 드리겠습니다.
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (2)
2393-2397: [버그] 수정요청 대기여부 키 불일치로 항상 false 가능성요청 저장은
final-contract:modification:{contractChatId}:{ownerId}키에 JSON(clauseOrder,requestData)로 저장되는데, 조회는{clauseOrder}를 키에 사용하고 있어 불일치합니다. 결과적으로 항상 false가 반환될 수 있습니다. 저장 포맷에 맞춰 키/값을 검증하도록 수정하세요.다음과 같이 키를 ownerId 기준으로 정정하고, 값에서 clauseOrder를 검증하세요.
- public boolean hasPendingModificationRequest(Long contractChatId, Integer clauseOrder) { - String redisKey = "final-contract:modification:" + contractChatId + ":" + clauseOrder; - return stringRedisTemplate.hasKey(redisKey); - } + public boolean hasPendingModificationRequest(Long contractChatId, Integer clauseOrder) { + ContractChat contractChat = contractChatMapper.findByContractChatId(contractChatId); + if (contractChat == null) return false; + String redisKey = + "final-contract:modification:" + contractChatId + ":" + contractChat.getOwnerId(); + String valueDataJson = stringRedisTemplate.opsForValue().get(redisKey); + if (valueDataJson == null) return false; + try { + com.fasterxml.jackson.databind.JsonNode rootNode = objectMapper.readTree(valueDataJson); + Integer storedClauseOrder = rootNode.get("clauseOrder").asInt(); + return clauseOrder.equals(storedClauseOrder); + } catch (Exception e) { + log.warn("수정 요청 데이터 파싱 실패: {}", e.getMessage()); + return false; + } + }
2700-2765: [버그] 최종 특약 삭제 수락 로직의 Redis 키/값 검증 오류요청 저장:
final-contract:deletion:{contractChatId}:{ownerId}키에 value로clauseOrder저장
수락 시: 현재는final-contract:deletion:{contractChatId}:{clauseOrder}키를 조회하고 value를 ownerId로 간주하여 비교합니다. 키와 값 모두 불일치입니다.다음과 같이 키를 ownerId로, 값 비교를 clauseOrder로 정정하세요.
- String redisKey = "final-contract:deletion:" + contractChatId + ":" + clauseOrder; - String storedOwnerId = stringRedisTemplate.opsForValue().get(redisKey); + String redisKey = + "final-contract:deletion:" + contractChatId + ":" + contractChat.getOwnerId(); + String storedClauseOrder = stringRedisTemplate.opsForValue().get(redisKey); - if (storedOwnerId == null) { + if (storedClauseOrder == null) { throw new BusinessException( ChatErrorCode.CONTRACT_END_REQUEST_NOT_FOUND, "삭제 요청이 존재하지 않습니다."); } - if (!storedOwnerId.equals(contractChat.getOwnerId().toString())) { + if (!storedClauseOrder.equals(String.valueOf(clauseOrder))) { throw new BusinessException( ChatErrorCode.CONTRACT_END_REQUEST_INVALID, "삭제 요청 정보가 유효하지 않습니다."); }
🧹 Nitpick comments (34)
src/main/java/org/scoula/domain/precontract/service/OwnerPreContractServiceImpl.java (8)
443-446: Mongo 선저장 타이밍 변경은 합리적. 다만 중복 저장/실패 복구 전략을 확인하세요.AI 메시징 전에
saveContractMongo를 수행하도록 한 변경은 일관성 있고 의존성 순서도 적절합니다. 다만 다음을 확인해 주세요:
- 이 메서드가 재호출되어도 중복 문서가 생기지 않도록 idempotent(예: (contractChatId, userId) 유니크 인덱스 + upsert)인지.
- 이후 메시징(
handleChatMessage,AiMessage) 실패 시 DB 상태를 롤백할지, 아니면 저장 성공을 우선시하고 메시징은 재시도/보상 흐름으로 처리할지 정책.필요 시 메시징 부분을 예외로부터 격리(try/catch)하여 DB 커밋과 분리하는 것도 고려해 주세요.
439-441: URL 콘텐츠가 하드코딩된 placeholder입니다
"계약 채팅방 URL"은 실제 사용자가 클릭할 수 있는 링크가 아닙니다. 프론트 베이스 URL을 환경변수/설정에서 주입받아 실제 경로로 구성해 주세요.예시:
- .content("계약 채팅방 URL") + // TODO: 실제 프론트 URL로 대체 필요 + .content(String.format("/contract-chat/%d", chatRoom.getChatRoomId()))클래스 필드에
@Value("${app.frontend.base-url}")등을 주입받아 절대경로로 구성할 것을 권장합니다.
508-508: 중복 호출 안전성: insert → save(upsert) 전환 제안
insert는 동일 문서 재저장 시 중복키 예외를 유발할 수 있습니다.save로 전환하여 upsert 동작을 허용하면 재실행/재시도에 안전합니다.- OwnerMongoDocument result = mongoRepository.insert(document); + // insert 대신 save로 upsert 동작을 허용 (중복 호출 안전) + OwnerMongoDocument result = mongoRepository.save(document);
584-598: 특약 추천 round 고정값(1L)은 중복 저장/충돌 위험
round를 1L로 고정하면 동일 계약 채팅에 대해 여러 번 호출 시 충돌하거나 덮어쓸 수 있습니다. 최신 round를 조회하여 +1 하거나, idempotency 키로 덮어쓰기를 명확히 해 주세요.원하시면 MongoTemplate/Repository 기반으로 최신 round 조회 → next round 계산 로직을 제안드립니다.
433-433: 반환값 미사용 호출
contractChatService.getContractChatStatus(contractChat.getStatus())의 반환값을 사용하지 않습니다. 사이드이펙트가 없다면 제거, 있다면 의미 있는 검증 로직으로 치환해 주세요.
53-53: 서비스 인터페이스 네이밍이 가이드와 불일치코딩 가이드에 따르면 서비스는
*ServiceInterface/*ServiceImpl페어를 사용해야 합니다. 현재 구현은implements OwnerPreContractService로 보입니다. 인터페이스를OwnerPreContractServiceInterface로, 구현체는 해당 인터페이스를 구현하도록 리네이밍을 검토해 주세요.
100-108: 주민번호 앞자리/주소 평문 저장은 규제/보안 리스크
ssnFront,addr1,name을 평문으로 저장하고 있습니다. 국내 규제 상 주민번호는 일부라도 엄격한 보호가 요구될 수 있습니다. 최소한 앞자리도 암호화/토큰화하거나, 별도 키 관리/접근통제를 강제하는 스토리지로 이동하는 방안을 검토해 주세요.보관 기간/마스킹 정책/접근 로그 요건 등 컴플라이언스 체크리스트가 필요하면 함께 정리해 드리겠습니다.
207-215: 복구 범위 upsert 시 삭제 동기화 누락 가능성요청 DTO에서 빠진 카테고리를 DB에서 제거하지 않으면 이전 선택이 잔존할 수 있습니다. 현재는 upsert만 수행하므로, 요청 리스트 기준으로 DB를 동기화(미포함 항목 삭제)하는 로직 추가를 권장합니다.
src/main/java/org/scoula/global/config/ServletConfig.java (2)
100-114: CORS 설정이 비활성화됨 — 필요 시 활성화하거나 전역 Bean으로 이동 권장현재
addCorsMappings전체가 주석이며, Nginx에서 CORS 헤더를 관리한다는 주석도 있습니다. 운영 구성에 따라 선택지가 갈립니다:
- Spring에서 CORS까지 관리: 아래 메서드를 활성화하고
CorsRegistryimport를 유지하세요.- Nginx에서 전담: 이 블록을 제거하고 import도 삭제해 중복 설정과 포맷 실패 가능성을 줄이세요.
또한
allowCredentials(true)를 사용할 때는 와일드카드 Origin(*)을 사용할 수 없는데, 현재는 명시적 Origin 리스트라서 OK입니다.메서드를 활성화하려면 아래 패치를 참고하세요:
-// @Override -// public void addCorsMappings(CorsRegistry registry) { -// registry.addMapping("/**") -// .allowedOrigins( -// "http://localhost:5173", -// "http://localhost:8080", -// "https://itzeep.ariogi.kr", -// "https://www.itzeep.ariogi.kr", -// "http://itzeep.ariogi.kr", -// "http://www.itzeep.ariogi.kr") -// .allowedMethods("GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH") -// .allowedHeaders("*") -// .allowCredentials(true) -// .maxAge(3600); -// } + @Override + public void addCorsMappings(CorsRegistry registry) { + registry.addMapping("/**") + .allowedOrigins( + "http://localhost:5173", + "http://localhost:8080", + "https://itzeep.ariogi.kr", + "https://www.itzeep.ariogi.kr", + "http://itzeep.ariogi.kr", + "http://www.itzeep.ariogi.kr") + .allowedMethods("GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH") + .allowedHeaders("*") + .allowCredentials(true) + .maxAge(3600); + }운영 환경에서 CORS를 어디서 책임질지 확정되지 않았다면 알려주세요. 선택에 맞춰 Spring 쪽 구성/코드 정리를 도와드리겠습니다.
17-17: 불필요한 CorsRegistry import 제거 제안CorsRegistry가 실제 코드에서 사용되지 않고 주석 처리된 CORS 메서드만 참조하므로 import를 제거해주세요.
(추후 CORS 적용 계획이 있다면 해당 메서드를 활성화하는 방향으로 정리해 주세요)
- 파일: src/main/java/org/scoula/global/config/ServletConfig.java (17행)
적용 가능한 패치:
--- a/src/main/java/org/scoula/global/config/ServletConfig.java +++ b/src/main/java/org/scoula/global/config/ServletConfig.java @@ -15,7 +15,6 @@ import org.springframework.web.multipart.support.StandardServletMultipartResolver; -import org.springframework.web.servlet.config.annotation.CorsRegistry; import org.springframework.web.servlet.config.annotation.EnableWebMvc; import org.springframework.web.servlet.config.annotation.ResourceHandlerRegistry;src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java (4)
26-33: System.err 디버그 출력 제거 또는 logger로 교체클래스 로딩/생성자에서
System.err.println은 운영 시 노이즈와 비용을 유발합니다.@Log4j2가 있으므로log.debug로 대체하거나 제거해 주세요.- static { - System.err.println("🚨🚨🚨 WebSocketConfig 클래스 로딩됨!"); - } - - public WebSocketConfig() { - System.err.println("🚨🚨🚨 WebSocketConfig 생성자 호출됨!"); - } + // 디버깅이 필요할 때만 활성화 + // static { + // log.debug("WebSocketConfig 클래스 로딩됨"); + // } + // public WebSocketConfig() { + // log.debug("WebSocketConfig 생성자 호출됨"); + // }
49-57: STOMP 엔드포인트 Origin 설정의 외부화 권장허용 Origin이 코드에 하드코딩되어 있어 운영 환경 추가/변경 시 재배포가 필요합니다.
application.yml에 배열 형태로 외부화하거나 프로파일별 분기 구성을 권장합니다. 또한 Nginx에서 CORS 헤더를 관리하더라도 Spring의 Origin 검증은 이대로 유지가 필요합니다.
104-109: 인증 토큰 타입 검토(선택)
UsernamePasswordAuthenticationToken(String, Object, Collection)생성자는authenticated=true인 인스턴스를 만듭니다. 검증 전 상태를 명확히 하거나 권한 부여가 필요 없다면 단순Principal구현체를 사용하거나, 검증 후에만 인증 토큰을 생성하도록 흐름을 분리하는 것을 고려하세요.Also applies to: 123-126
61-140: JwtUtil Bean 재사용 권장프로젝트에 이미
org.scoula.global.auth.jwt.JwtUtilBean이 존재합니다. WebSocketConfig에서 직접 Base64 디코딩·파싱 로직 대신 해당 Bean을 주입하여 토큰 유효성 검사 및 사용자 정보 추출을 일관되게 처리하세요.• 파일: src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java
- 생성자(또는 필드) 주입으로
JwtUtil jwtUtil추가preSend()내부의 직접 디코딩·파싱 로직 제거jwtUtil.getTokenInfo(token)호출 후TokenInfo.getSubject()로 사용자 설정이렇게 하면 유지보수성과 일관성이 개선됩니다.
src/main/java/org/scoula/domain/chat/controller/ContractChatControllerImpl.java (2)
7-7: 불필요한 Objects 임포트
Objects클래스가 Line 437에서 한 번만 사용되고 있습니다. 기존의equals()메서드를 직접 사용하는 방식으로도 충분하므로, 이 임포트는 선택사항입니다.
304-329: WebSocket 입장 로직 개선WebSocket 계약 채팅방 입장 메서드에 상세한 로깅이 추가되었습니다. 다만 몇 가지 개선사항이 있습니다:
- Line 318: 에러 로그 후 조용히 리턴하는 대신, 사용자에게 에러를 알려주는 것이 좋습니다
- Principal null 체크는 이미 Line 308에서 로깅하고 있으므로, 실제 사용 시점에서 검증이 필요할 수 있습니다
if (userId == null || contractChatId == null) { log.error("필수 파라미터 누락 - userId: {}, contractChatId: {}", userId, contractChatId); + // 에러 메시지를 클라이언트에 전송 + Map<String, Object> errorInfo = Map.of( + "error", "MISSING_PARAMETERS", + "message", "필수 파라미터가 누락되었습니다." + ); + messagingTemplate.convertAndSendToUser( + principal != null ? principal.getName() : "unknown", + "/queue/contract/error", + errorInfo + ); return; }src/main/java/org/scoula/domain/chat/service/ChatServiceImpl.java (1)
842-848: URL 문자열 연결 대신 UriComponentsBuilder 사용 권장 (인코딩/슬래시/쿼리 안전성 개선)현재 문자열 더하기 방식은 누락/중복 슬래시, 쿼리 파라미터 인코딩 누락 등 엣지 케이스 위험이 있습니다. Spring의 UriComponentsBuilder로 대체하면 안전성과 가독성이 좋아집니다.
다음 변경을 제안합니다:
- String contractChatUrl = - URL - + PRECONTRACTURL - + (contractChatRoomId.toString()) - + BUYERURL - + "&homeId=" - + (originalChatRoom.getHomeId()); + String contractChatUrl = org.springframework.web.util.UriComponentsBuilder + .fromHttpUrl(URL) + .pathSegment("pre-contract", contractChatRoomId.toString(), "buyer") + .queryParam("step", 1) + .queryParam("homeId", originalChatRoom.getHomeId()) + .build() + .toUriString();추가로 필요한 import:
import org.springframework.web.util.UriComponentsBuilder;src/main/java/org/scoula/domain/contract/repository/ContractMongoRepository.java (1)
106-122: System.out.println 제거 및 로깅 일원화콘솔 출력 대신 로거를 사용해 주세요. 저장소 레이어에서도 운영 환경 로깅 일관성이 중요합니다.
다음과 같이 교체를 제안합니다:
- System.out.println("특약 내용 삭제 완료 - contractChatId: " + contractChatId); + log.info("특약 내용 삭제 완료 - contractChatId={}", contractChatId);클래스 상단에 로거 추가(외부 범위 변경):
import org.slf4j.Logger; import org.slf4j.LoggerFactory; @Repository public class ContractMongoRepository { private static final Logger log = LoggerFactory.getLogger(ContractMongoRepository.class); ... }src/main/java/org/scoula/domain/contract/service/ContractFixServiceInterface.java (1)
7-7: 반환형은 WrapperVoid보단 기본형void사용 권장
Void는 일반적으로 의미 없는 반환을 객체로 다뤄야 할 때만 사용합니다. 여기서는void가 더 적절하고 호출부에도 편합니다.다음 변경을 제안합니다:
- Void saveSpecialContract(Long contractChatId, Long userId); + void saveSpecialContract(Long contractChatId, Long userId);src/main/resources/org/scoula/domain/precontract/mapper/TenantPreContractMapper.xml (1)
262-268: selectRentTypeAll resultType 대소문자 통일 요청
MyBatis 내 동일 파일의selectRentType이resultType="String"을 사용하고 있으므로, 일관성을 위해selectRentTypeAll도resultType="String"으로 변경해주세요.
인터페이스TenantPreContractMapper에서@Param("contractChatId")·@Param("userId")매핑은 이미 올바르게 설정되어 있습니다.
- 변경 위치
- 파일:
src/main/resources/org/scoula/domain/precontract/mapper/TenantPreContractMapper.xml- 대상 코드: lines 262–268
- <select id="selectRentTypeAll" resultType="string"> + <select id="selectRentTypeAll" resultType="String">src/main/java/org/scoula/domain/precontract/mapper/TenantPreContractMapper.java (1)
42-44: 메서드 목적 구분을 위한 JavaDoc 추가 및 @param 변수명 통일 권장
TenantPreContractMapper에 정의된 두 메서드(selectRentTypevsselectRentTypeAll)는 호출 위치·범위가 다르므로, 혼동 방지를 위해 아래와 같이 리팩토링을 제안합니다.
JavaDoc으로 용도 명시
interface TenantPreContractMapper { - Optional<String> selectRentType( - @Param("contractChatId") Long contractChatId, @Param("userId") Long userid); + /** + * 사전계약(tenant/owner) 컨텍스트에서 전/월세 타입 조회. + */ + Optional<String> selectRentType( + @Param("contractChatId") Long contractChatId, @Param("userId") Long userId); - Optional<String> selectRentTypeAll( - @Param("contractChatId") Long contractChatId, @Param("userId") Long userid); + /** + * 계약 전체(모든 사전계약 데이터) 컨텍스트에서 전/월세 타입 조회. + * ContractServiceImpl에서 단일 호출 시 사용. + */ + Optional<String> selectRentTypeAll( + @Param("contractChatId") Long contractChatId, @Param("userId") Long userId); }
userid→userId로 변수명 통일
• 파일:src/main/java/org/scoula/domain/precontract/mapper/TenantPreContractMapper.java
• 변경 대상 메서드:selectRentType,selectRentTypeAllsrc/main/java/org/scoula/domain/contract/controller/ContractController.java (1)
122-131: 인증 주입 방식 통일 및 응답 DTO 사용 권장현재
ContractController인터페이스(src/main/java/org/scoula/domain/contract/controller/ContractController.java)와 구현체(ContractControllerImpl.java)에서
Authentication authentication을 사용 중이며, 타 엔드포인트들은@AuthenticationPrincipal CustomUserDetails userDetails를 사용하고 있습니다.
컨트롤러 전반의 일관성을 위해 아래와 같이 시그니처를 변경해 주세요:- ResponseEntity<ApiResponse<String>> requestFinalContract( - @PathVariable Long contractChatId, Authentication authentication); + ResponseEntity<ApiResponse<String>> requestFinalContract( + @PathVariable Long contractChatId, + @AuthenticationPrincipal CustomUserDetails userDetails); - ResponseEntity<ApiResponse<Map<String, Object>>> acceptFinalContract( - @PathVariable Long contractChatId, - @RequestBody FinalContractDeletionResponseDto responseDto, - Authentication authentication); + ResponseEntity<ApiResponse<Map<String, Object>>> acceptFinalContract( + @PathVariable Long contractChatId, + @RequestBody FinalContractDeletionResponseDto responseDto, + @AuthenticationPrincipal CustomUserDetails userDetails);
- Map<String, Object> 대신 명시적 응답 DTO 사용 권장(가독성·타입 안전성↑)
- 필요 시
FinalContractAcceptResponseDto초안 제공 가능합니다.src/main/java/org/scoula/domain/contract/dto/LegalityDTO.java (1)
32-37: validationStatus는 Enum 도입 고려validationStatus를 자유 문자열 대신 Enum으로 모델링하면 오탈자/상태 폭주를 방지할 수 있습니다. 예: PENDING, PASSED, FAILED 등.
예시:
public enum ValidationStatus { PENDING, PASSED, FAILED }@JsonProperty("validation_status") private ValidationStatus validationStatus;다운스트림/직렬화 포맷 영향이 있으므로 단계적 도입을 권장합니다.
src/main/java/org/scoula/domain/contract/service/ContractFixService.java (2)
72-77: assert 사용 제거 및 로그 민감도 조정 제안
- assert로 null을 강제하는 것은 프로덕션에서 비활성화되어 무의미하거나, 활성화된 경우 불필요한 AssertionError를 유발할 수 있습니다. 아래 if 블록에서 이미 null 체크를 하므로 assert는 제거하세요.
- 응답 전체를 warn 레벨로 출력하면 과도한 정보(민감정보 포함)가 로그로 남을 수 있습니다. 상태코드 중심의 debug 로그로 낮추는 것을 권장합니다.
다음과 같이 정리하는 것을 제안합니다:
- LegalityDTO res = response.getBody(); - assert res != null; - log.warn("AI 응답 값 확인: {}", res.toString()); - - log.warn("AI 응답 헤더 확인: {}", response.getStatusCode()); + LegalityDTO res = response.getBody(); + // 응답은 아래 조건문에서 null 체크합니다. assert 사용은 지양합니다. + log.debug("AI 응답 상태: {}", response.getStatusCode());
94-97: 예외 로그에 스택트레이스 포함현재는 메시지 문자열만 로깅하여 원인 파악이 어려울 수 있습니다. 스택트레이스를 포함해 주세요.
- } catch (Exception e) { - log.error(e.getMessage()); - throw new BusinessException(ContractException.CONTRACT_AI_SERVER_ERROR, e); - } + } catch (Exception e) { + log.error("AI 서버 호출/파싱 실패", e); + throw new BusinessException(ContractException.CONTRACT_AI_SERVER_ERROR, e); + }src/main/java/org/scoula/domain/contract/service/ContractServiceImpl.java (3)
234-241: 전/월세 타입 매핑의 하드코딩 최소화문자열 비교 대신 enum 또는 상수 사용을 권장합니다. 또한 NPE를 방지하려면 equals 비교 방향을 상수 우선으로 유지하세요.
- String rentTypeKr; - if(rentType.equals("JEONSE")){ + String rentTypeKr; + if ("JEONSE".equals(rentType)) { rentTypeKr = "전세"; }else{ rentTypeKr="월세"; }추가로, 향후 타입이 늘어날 경우 switch 문이나 매핑 테이블(Map<String, String>)로 확장성을 확보하는 것이 좋습니다.
149-154: 서비스 레이어에서 Thread.sleep 사용 지양블로킹 대기(2초)는 처리량과 응답성에 악영향을 줍니다. 비동기 이벤트(메시지 큐/스케줄링)나 클라이언트 측 지연 표시로 대체를 권장합니다.
Also applies to: 199-211, 375-383
278-288: Redis 저장 포맷 변경(CSV) 검증 완료 및 TTL 적용 권장전역 검색 결과
contract:payment:{contractChatId}키를 읽는 코드는 모두ContractServiceImpl내에 있으며, JSON을 기대하는 별도 소비자는 없습니다.
임시 합의값 특성 상 데이터 유실·방치 방지를 위해 TTL을 추가 적용할 것을 권장합니다.
- 파일:
src/main/java/org/scoula/domain/contract/service/ContractServiceImpl.java
– saveDepositPrice, updateDepositPrice, 삭제 로직 모두 CSV("deposit,monthly")로 정상 처리됨- 권장 적용 예시:
- stringRedisTemplate.opsForValue().set(redisKey, paymentValue); + stringRedisTemplate.opsForValue().set(redisKey, paymentValue, Duration.ofMinutes(30));src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (3)
2475-2624: 적법성 검사 결과 과다 로깅(PII/민감정보) 가능성위반 세부내용(조항 원문, 개선안 등)을 info 레벨로 상세히 로그에 남기고 있습니다. 운영 환경에서는 debug 레벨로 낮추거나, 요약/マ스킹 처리로 전환하세요. 프런트 전달은 메시지 전송(AiMessage*)로 충분합니다.
627-638: Redis 기반 온라인 상태로 통일 권장(중복 상태 관리 제거)
contractChatOnlineUsers인메모리 맵을 online/offline에서 부분적으로만 사용하고 있어(online 세팅은 미사용), 상태 관리가 이원화되어 혼란을 유발할 수 있습니다. Redis 기반으로 일원화하고 인메모리 경로는 제거하는 것을 권장합니다.Also applies to: 641-652
497-513: Presence TTL/유실 복구 검토입장/퇴장 시 Redis 셋을 갱신하지만, 비정상 종료(브라우저 강제 종료 등) 복구 전략이 없습니다. 주기적 하트비트/TTL 기반 만료, 또는 세션 종료 훅과의 연동을 검토하세요.
Also applies to: 519-526
src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java (3)
214-230: 스웨거 문서화 어노테이션 추가 권장(가이드라인 준수)컨트롤러 엔드포인트는 Swagger 문서화를 권장합니다. 최소한 신규 엔드포인트에
@Operation을 추가하세요. 클래스 레벨@Tag도 함께 권장합니다.+import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.tags.Tag; @@ -@RestController +@RestController +@Tag(name = "Contract", description = "계약 진행/특약/적법성 관련 API") @@ - @PostMapping("/specialContract/final-request") + @Operation(summary = "최종 특약 확정 요청", description = "임대인이 임차인에게 최종 특약 확정을 요청합니다.") + @PostMapping("/specialContract/final-request") public ResponseEntity<ApiResponse<String>> requestFinalContract( @PathVariable Long contractChatId, Authentication authentication) {
232-253: 스웨거 문서화 어노테이션 추가 권장(수락/거절 엔드포인트)동일하게 수락/거절 엔드포인트에도
@Operation을 부여해 주세요.- @PostMapping("/specialContract/final-accept") + @Operation(summary = "최종 특약 확정 수락/거절", description = "임차인이 최종 특약 확정 요청을 수락 또는 거절합니다.") + @PostMapping("/specialContract/final-accept") public ResponseEntity<ApiResponse<Map<String, Object>>> acceptFinalContract(
233-239: 요청 DTO 명확성 개선 제안
final-accept요청 바디로FinalContractDeletionResponseDto를 재사용하고 있습니다. 의미적으로 다른 목적의 DTO를 재사용하면 혼동을 유발할 수 있으므로,FinalContractAcceptRequestDto처럼 목적에 맞는 전용 DTO를 정의하는 것을 권장합니다.원하시면 전용 DTO와 매핑 코드 패치를 생성해 드리겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
src/main/java/org/scoula/domain/chat/controller/ContractChatControllerImpl.java(5 hunks)src/main/java/org/scoula/domain/chat/service/ChatServiceImpl.java(1 hunks)src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java(9 hunks)src/main/java/org/scoula/domain/chat/service/ContractChatServiceInterface.java(1 hunks)src/main/java/org/scoula/domain/contract/controller/ContractController.java(2 hunks)src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java(8 hunks)src/main/java/org/scoula/domain/contract/dto/LegalityDTO.java(2 hunks)src/main/java/org/scoula/domain/contract/repository/ContractMongoRepository.java(3 hunks)src/main/java/org/scoula/domain/contract/service/ContractFixService.java(1 hunks)src/main/java/org/scoula/domain/contract/service/ContractFixServiceInterface.java(1 hunks)src/main/java/org/scoula/domain/contract/service/ContractService.java(0 hunks)src/main/java/org/scoula/domain/contract/service/ContractServiceImpl.java(11 hunks)src/main/java/org/scoula/domain/precontract/mapper/TenantPreContractMapper.java(2 hunks)src/main/java/org/scoula/domain/precontract/service/OwnerPreContractServiceImpl.java(1 hunks)src/main/java/org/scoula/domain/precontract/service/PreContractDataServiceImpl.java(1 hunks)src/main/java/org/scoula/domain/precontract/service/TenantPreContractServiceImpl.java(4 hunks)src/main/java/org/scoula/global/config/ServletConfig.java(2 hunks)src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java(3 hunks)src/main/resources/org/scoula/domain/precontract/mapper/OwnerPreContractMapper.xml(1 hunks)src/main/resources/org/scoula/domain/precontract/mapper/TenantPreContractMapper.xml(3 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/org/scoula/domain/contract/service/ContractService.java
🧰 Additional context used
📓 Path-based instructions (6)
src/main/java/org/scoula/**/service/*Service{Interface,Impl}.java
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use interface-implementation pattern for services:
*ServiceInterface.java(interface) and*ServiceImpl.java(implementation)
Files:
src/main/java/org/scoula/domain/chat/service/ChatServiceImpl.javasrc/main/java/org/scoula/domain/precontract/service/PreContractDataServiceImpl.javasrc/main/java/org/scoula/domain/chat/service/ContractChatServiceInterface.javasrc/main/java/org/scoula/domain/contract/service/ContractFixServiceInterface.javasrc/main/java/org/scoula/domain/precontract/service/OwnerPreContractServiceImpl.javasrc/main/java/org/scoula/domain/precontract/service/TenantPreContractServiceImpl.javasrc/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.javasrc/main/java/org/scoula/domain/contract/service/ContractServiceImpl.java
src/main/resources/org/scoula/domain/*/mapper/*Mapper.xml
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/main/resources/org/scoula/domain/*/mapper/*Mapper.xml: Place MyBatis XML mappers atsrc/main/resources/org/scoula/domain/*/mapper/*Mapper.xml
Name MyBatis XML mappers*Mapper.xmlto match their corresponding*Mapper.javainterfaces
Files:
src/main/resources/org/scoula/domain/precontract/mapper/OwnerPreContractMapper.xmlsrc/main/resources/org/scoula/domain/precontract/mapper/TenantPreContractMapper.xml
src/main/java/org/scoula/domain/*/repository/*Repository.java
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place MongoDB repositories under
src/main/java/org/scoula/domain/*/repository/*Repository.java
Files:
src/main/java/org/scoula/domain/contract/repository/ContractMongoRepository.java
src/main/java/org/scoula/**/controller/*Controller{,Impl}.java
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use interface-implementation pattern for controllers:
*Controller.java(interface) and*ControllerImpl.java(implementation)
Files:
src/main/java/org/scoula/domain/contract/controller/ContractController.javasrc/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.javasrc/main/java/org/scoula/domain/chat/controller/ContractChatControllerImpl.java
src/main/java/org/scoula/**/controller/**/*.java
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/main/java/org/scoula/**/controller/**/*.java: All controller endpoints should return a consistentApiResponse<T>wrapper
Add Swagger annotations to controller endpoints for API documentation
Files:
src/main/java/org/scoula/domain/contract/controller/ContractController.javasrc/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.javasrc/main/java/org/scoula/domain/chat/controller/ContractChatControllerImpl.java
src/main/java/org/scoula/domain/*/mapper/*Mapper.java
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/main/java/org/scoula/domain/*/mapper/*Mapper.java: Define MyBatis mapper interfaces undersrc/main/java/org/scoula/domain/*/mapper/*Mapper.java
Annotate MyBatis mapper interfaces with@Mapper
Files:
src/main/java/org/scoula/domain/precontract/mapper/TenantPreContractMapper.java
🧠 Learnings (5)
📚 Learning: 2025-08-04T00:51:44.587Z
Learnt from: Whatdoyumin
PR: ITZEEP/backend#40
File: src/main/resources/org/scoula/domain/precontract/mapper/OwnerPreContractMapper.xml:188-192
Timestamp: 2025-08-04T00:51:44.587Z
Learning: In the ITZEEP backend codebase, the owner_precontract_check table uses contract_chat_id as both primary key (PK) and foreign key (FK), so selectOwnerPrecheckId correctly returns contract_chat_id for use in upsertRestoreScope operations. This is confirmed by the join pattern "LEFT JOIN owner_restore_scope ors ON opc.contract_chat_id = ors.owner_precheck_id" in the mapper files.
Applied to files:
src/main/resources/org/scoula/domain/precontract/mapper/OwnerPreContractMapper.xml
📚 Learning: 2025-08-04T00:51:44.587Z
Learnt from: Whatdoyumin
PR: ITZEEP/backend#40
File: src/main/resources/org/scoula/domain/precontract/mapper/OwnerPreContractMapper.xml:188-192
Timestamp: 2025-08-04T00:51:44.587Z
Learning: In the ITZEEP backend codebase, the owner_precontract_check table uses contract_chat_id as both primary key (PK) and foreign key (FK), so selectOwnerPrecheckId correctly returns contract_chat_id for use in upsertRestoreScope operations.
Applied to files:
src/main/resources/org/scoula/domain/precontract/mapper/OwnerPreContractMapper.xml
📚 Learning: 2025-08-12T03:59:30.512Z
Learnt from: CR
PR: ITZEEP/backend#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-12T03:59:30.512Z
Learning: Applies to src/main/java/org/scoula/domain/*/repository/*Repository.java : Place MongoDB repositories under `src/main/java/org/scoula/domain/*/repository/*Repository.java`
Applied to files:
src/main/java/org/scoula/domain/contract/repository/ContractMongoRepository.java
📚 Learning: 2025-08-12T03:59:30.512Z
Learnt from: CR
PR: ITZEEP/backend#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-12T03:59:30.512Z
Learning: Applies to src/main/java/org/scoula/**/service/*Service{Interface,Impl}.java : Use interface-implementation pattern for services: `*ServiceInterface.java` (interface) and `*ServiceImpl.java` (implementation)
Applied to files:
src/main/java/org/scoula/domain/contract/service/ContractFixServiceInterface.java
📚 Learning: 2025-08-12T03:59:30.512Z
Learnt from: CR
PR: ITZEEP/backend#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-12T03:59:30.512Z
Learning: Applies to src/main/java/org/scoula/**/controller/*Controller{,Impl}.java : Use interface-implementation pattern for controllers: `*Controller.java` (interface) and `*ControllerImpl.java` (implementation)
Applied to files:
src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java
🧬 Code Graph Analysis (3)
src/main/java/org/scoula/domain/contract/service/ContractFixService.java (1)
src/main/java/org/scoula/global/common/exception/BusinessException.java (1)
BusinessException(4-21)
src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java (1)
src/main/java/org/scoula/global/common/exception/BusinessException.java (1)
BusinessException(4-21)
src/main/java/org/scoula/domain/contract/dto/LegalityDTO.java (3)
src/main/java/org/scoula/domain/contract/dto/ContractDTO.java (1)
Data(57-65)src/main/java/org/scoula/domain/contract/document/ContractMongoDocument.java (1)
Data(66-74)src/main/java/org/scoula/domain/contract/dto/SpecialContractUpdateDTO.java (1)
Data(19-27)
🪛 GitHub Actions: CI
src/main/java/org/scoula/global/config/ServletConfig.java
[error] 1-1: spotlessJavaCheck failed. Formatting violations detected in ServletConfig.java. Run './gradlew :spotlessApply' to fix code style issues in this file.
src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java
[error] 1-1: spotlessJavaCheck failed. Formatting violations detected in ContractControllerImpl.java. Run './gradlew :spotlessApply' to fix code style issues in this file.
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java
[error] 2872-2919: spotlessJavaCheck failed. Formatting violations detected in ContractChatServiceImpl.java. Run './gradlew :spotlessApply' to fix code style issues in this file.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CodeQL Security Analysis (java)
🔇 Additional comments (22)
src/main/java/org/scoula/global/config/ServletConfig.java (1)
1-115: Spotless 포맷 실패 처리CI 로그에 따르면 이 파일에서 Spotless 포맷 위반이 있습니다. 주석 블록과 import 섹션 정리 후에도 동일하다면 로컬에서
:spotlessApply를 실행해 자동 정렬을 적용해 주세요. 포맷 규칙 상 주석의 들여쓰기/빈 줄/임포트 순서가 수정될 수 있습니다.위 코멘트 상단의 스크립트를 실행해 결과를 공유해 주시면, 남은 포맷 이슈를 정확히 짚어드리겠습니다.
src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java (1)
37-40: 메시지 브로커 설정 확장 LGTM
/topic,/queue활성화와/user목적지 프리픽스 설정은 사용자 개별 큐(1:1 DM) 및 브로드캐스트 시나리오에 적절합니다. 문제 없습니다.src/main/java/org/scoula/domain/chat/controller/ContractChatControllerImpl.java (2)
437-437: LGTM! 역할 판별 로직 개선
Objects.equals()를 사용하여 null-safe한 비교를 수행하도록 개선되었습니다. 이는 NPE를 방지하는 좋은 방법입니다.
975-996: getContractStatus 메서드가 여전히 매핑되어 사용 중입니다.ContractChatController 인터페이스에 @GetMapping("/{contractChatId}/status") 어노테이션이 선언되어 있어, 해당 엔드포인트가 정상적으로 노출되고 호출 가능합니다.
따라서 이 메서드는 미사용이 아니며 삭제할 필요가 없습니다.
- src/main/java/org/scoula/domain/chat/controller/ContractChatController.java:203 (@GetMapping("/{contractChatId}/status") on getContractStatus)
- src/main/java/org/scoula/domain/chat/controller/ContractChatControllerImpl.java:975 (메서드 구현부)
Likely an incorrect or invalid review comment.
src/main/resources/org/scoula/domain/precontract/mapper/OwnerPreContractMapper.xml (1)
146-147: 계약 단위로 본인인증 스코프를 좁힌 쿼리 보강, 성능 인덱스 확인 권장
AND iv.contract_id = cc.contract_chat_id추가로 동일 사용자의 타 계약 건이 섞이는 문제를 방지합니다. 올바른 보강입니다. 다만 다음을 확인해 주세요:
- identity_verification 테이블에
(contract_id, user_id)복합 인덱스가 없다면 추가를 고려하세요. 해당 조건 조합이 이 파일 전반에서 빈번히 사용됩니다.- 상단
updateIdentityVerification는contract_step = 'START'로 필터링하지만, 여기서는 단계 필터가 없습니다. 계약 단계별 중복 레코드가 쌓일 수 있다면 동일 계약의 최신/유일 레코드로 제한하는 추가 조건이 필요한지 검토 바랍니다.src/main/java/org/scoula/domain/contract/repository/ContractMongoRepository.java (2)
4-4: 필요한 컬렉션 초기화를 위한 ArrayList import 추가 적절
clearSpecialContracts에서 빈 리스트 설정을 위해 ArrayList import가 필요합니다. 변경 타당합니다.
92-97: 특약 order 오프바이원 제거 확인 및 운영 데이터 정합성 검증 필요
변경 사항
fc.getOrder() + 1→fc.getOrder()로 수정되어 0-기반 인덱싱을 가정합니다.
ContractMongoRepository 에서도existingClauses.get(order)으로 0-기반 접근하며, DTO·서비스 전반의.order(getOrder())사용 패턴이 모두 일관된 것을 확인했습니다.검증 포인트
운영 중인 MongoDB에 저장된specialContracts.order값이 1-기반(최소값이 1)으로 기록돼 있다면, 업데이트 시 오프바이원(one-off-by-one) 문제가 발생할 수 있습니다.분포 확인 방법
Mongo Shell 에서 아래 명령으로 기존 값의 분포를 확인하세요.db.contracts.distinct('specialContracts.order')0이 포함돼 있는지, 최소·최대값 대비 항목 수가 예상과 일치하는지 점검이 필요합니다.
데이터 마이그레이션 예시
1-기반으로 저장된 값을 0-기반으로 변환하려면, 예를 들어 다음 스크립트를 사용해 보세요.db.contracts.find().forEach(doc => { doc.specialContracts = doc.specialContracts.map(sc => ({ ...sc, order: sc.order - 1 })); db.contracts.save(doc); });운영 데이터의 기준(order 시작값 및 분포)을 반드시 검증하고, 필요 시 마이그레이션 계획을 수립·실행해주세요.
src/main/java/org/scoula/domain/contract/service/ContractFixServiceInterface.java (1)
1-10: 서비스 인터페이스 구조/위치 적합서비스 인터페이스-구현 분리 규칙을 충족하고, 패키지/파일 네이밍도 가이드에 부합합니다.
src/main/resources/org/scoula/domain/precontract/mapper/TenantPreContractMapper.xml (2)
30-37: 계약별 본인인증 레코드로 스코프 제한 — 중복 방지에 효과적
AND iv.contract_id = #{contractChatId}추가로 다른 계약의 본인인증 레코드가 섞이는 문제를 방지합니다.SELECT DISTINCT와 함께 적절한 보강입니다. 성능을 위해(user_id, contract_id)복합 인덱스 존재 여부도 확인해 주세요.
57-59: 비기능 변경 (공백 라인 추가)기능 영향이 없어 코멘트 생략합니다.
src/main/java/org/scoula/domain/precontract/service/PreContractDataServiceImpl.java (1)
91-95: 검증 완료: selectIdentityId 호출 및 MyBatis 매핑 정상 반영
검토 결과 tenantMapper.selectIdentityId(contractChatId, userId) 및 ownerMapper.selectIdentityId(contractChatId) 호출부가 모두 업데이트되었고, XML 매퍼에서도 #{userId}, #{contractChatId} 파라미터가 올바르게 반영된 것을 확인했습니다.
이상 없으므로 코드 변경을 승인합니다.src/main/java/org/scoula/domain/precontract/mapper/TenantPreContractMapper.java (1)
27-29: selectIdentityId 시그니처 확장 완료 — Tenant 매퍼 일관성 확인됨
TenantPreContractMapper XML 매핑에서contract_chat_id와user_id가 모두 반영되었으며, 아래 호출부가 모두 새로운 시그니처(contractChatId, userId)를 사용하고 있습니다:
- src/main/java/org/scoula/domain/precontract/service/TenantPreContractServiceImpl.java:88
- src/main/java/org/scoula/domain/precontract/service/PreContractDataServiceImpl.java:91
src/main/java/org/scoula/domain/precontract/service/TenantPreContractServiceImpl.java (1)
86-91: identity 조회 시 복합 조건 적용 확인 및 인덱스 검증 필요
- TenantPreContractMapper.xml 의
selectIdentityId쿼리에서
FROM contract_chat cc INNER JOIN identity_verification iv ON cc.owner_id = iv.user_idWHERE cc.contract_chat_id = #{contractChatId}와iv.user_id = #{userId}복합 조건이 올바르게 적용되어 있는 것이 확인되었습니다.- 다만, 레포에 스키마(SQL 마이그레이션) 파일이 포함되어 있지 않아
contract_chat(contract_chat_id, user_id)복합 인덱스 혹은 개별 인덱스 존재 여부- 대량 트래픽 환경에서 조회 성능
를 자동 확인할 수 없습니다.
→ DB 마이그레이션 또는 실제 스키마 정의에서 관련 인덱스가 적절히 설정되어 있는지 검증해 주세요.src/main/java/org/scoula/domain/contract/dto/LegalityDTO.java (2)
38-39: violations 기본값 초기화 적절@Builder.Default로 null 방지한 것은 직렬화/클라이언트 단 처리에 유리합니다. LGTM.
20-23: LegalityDTO 데이터 구조 재배치 검증 완료 — 모든 호출부가 새 Payload 구조 사용 중
getData().getValidationStatus(),getTotalViolations(),getViolations()등이ContractChatServiceImpl등에서 정상적으로 새 중첩 구조로 호출되고 있음을 확인했습니다. 추가적인 다운스트림 수정이 필요 없습니다.
- src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (lines 2510–2523):
legalityDTO.getData().getValidationStatus(),getTotalViolations(),getViolations()src/main/java/org/scoula/domain/contract/service/ContractFixService.java (1)
115-153: 사용자 검증 로직 일관성 양호소유자/임차인 분기 및 매퍼 기반의 이중 확인 흐름이 명확하고 예외 코드 사용도 일관됩니다. 별도 이슈 없습니다.
src/main/java/org/scoula/domain/contract/service/ContractServiceImpl.java (2)
146-148: STEP0 상태 전환 시점 추가: 적절함초기 안내 메시지 직후 STEP0로 상태를 기록하는 변경은 클라이언트 동기화 측면에서 더 예측 가능합니다.
166-173: 3단계(특약 조율) 안내 메시지 추가: 가독성/도메인 친화적사용자 안내 메시지가 명확해졌습니다. UX 향상에 기여합니다.
src/main/java/org/scoula/domain/chat/service/ContractChatServiceInterface.java (1)
305-310: 새 인터페이스 메서드 추가 적정최종 특약 확정 요청/수락 API가 인터페이스로 노출되어 구현체와 컨트롤러 연동이 명확합니다.
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (2)
2872-2919: Spotless 포맷팅 실패: 스타일 정리 필요CI가 이 범위에서 spotlessJavaCheck 실패를 보고했습니다.
./gradlew :spotlessApply를 실행해 포맷팅을 일괄 적용해 주세요.
104-127: 오프라인 시 메시지 미저장 정책 확인 필요상대방 오프라인이면 에러를 발신자에게만 보내고 메시지는 저장하지 않습니다. 의도된 UX라면 좋으나, 대화 흐름 보존이 필요한 요구사항이라면 큐잉/지연전송 옵션을 검토하세요.
src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java (1)
1-1: Spotless 포맷팅 실패: 스타일 정리 필요CI가 이 파일에서 spotlessJavaCheck 실패를 보고했습니다.
./gradlew :spotlessApply로 포맷팅을 적용해 주세요.
| import org.springframework.web.bind.annotation.*; | ||
|
|
||
| import lombok.extern.slf4j.Slf4j; | ||
| import lombok.extern.log4j.Log4j2; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
로깅 프레임워크 변경: Slf4j → Log4j2
Slf4j에서 Log4j2로 로깅 프레임워크를 변경했습니다. 프로젝트 전체에서 일관된 로깅 프레임워크를 사용하는지 확인이 필요합니다.
Also applies to: 38-38
🏁 Script executed:
#!/bin/bash
# Log4j2와 Slf4j 사용 현황 확인
echo "=== Checking logging framework usage ==="
echo "Log4j2 annotations:"
rg -n "@Log4j2" --type java | head -20
echo ""
echo "Slf4j annotations:"
rg -n "@Slf4j" --type java | head -20Length of output: 2293
전체 로깅 어노테이션 통일 필요: Slf4j → Log4j2
프로젝트 전반에서 일관된 로깅 프레임워크 사용을 위해, 아래 4개 클래스에 남아 있는 @Slf4j 어노테이션을 @Log4j2로 변경해 주세요.
- src/main/java/org/scoula/global/common/controller/EncryptionKeyCheckController.java (23행)
- src/main/java/org/scoula/auth/util/S3Uploader.java (24행)
- src/main/java/org/scoula/domain/chat/repository/ContractChatMessageRepository.java (16행)
- src/main/java/org/scoula/domain/fraud/service/FraudRiskServiceImpl.java (44행)
| // 디버깅용 임시 엔드포인트 | ||
| @PostMapping("/{contractChatId}/debug/enter") | ||
| public ResponseEntity<ApiResponse<String>> debugEnterContractChatRoom( | ||
| @PathVariable Long contractChatId, Authentication authentication) { | ||
| try { | ||
| Long userId = getUserIdFromAuthentication(authentication); | ||
| log.info("=== HTTP 디버그 계약 채팅방 입장 ==="); | ||
| log.info("contractChatId: {}, userId: {}", contractChatId, userId); | ||
|
|
||
| contractChatService.enterContractChatRoom(contractChatId, userId); | ||
|
|
||
| return ResponseEntity.ok(ApiResponse.success("입장 완료")); | ||
| } catch (Exception e) { | ||
| log.error("디버그 입장 실패", e); | ||
| return ResponseEntity.badRequest().body(ApiResponse.error("입장 실패: " + e.getMessage())); | ||
| } | ||
| } |
There was a problem hiding this comment.
디버깅 엔드포인트 보안 검토 필요
디버깅용 임시 엔드포인트가 추가되었습니다. 프로덕션 환경에 배포되지 않도록 주의가 필요합니다:
- 프로필 기반 조건부 활성화 고려
- 접근 권한 제한 필요
- 배포 전 제거 확인
+import org.springframework.context.annotation.Profile;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
// 디버깅용 임시 엔드포인트
+@Profile({"dev", "local"}) // 개발/로컬 환경에서만 활성화
@PostMapping("/{contractChatId}/debug/enter")
public ResponseEntity<ApiResponse<String>> debugEnterContractChatRoom(또는 프로덕션 배포 전에 이 엔드포인트를 완전히 제거하는 것을 권장합니다.
🤖 Prompt for AI Agents
In
src/main/java/org/scoula/domain/chat/controller/ContractChatControllerImpl.java
around lines 957 to 973, the temporary debug POST endpoint for entering contract
chat rooms is a security risk for production; either remove this endpoint before
deploy or guard it so it cannot run in production: restrict activation to
non-production profiles or a specific debug property (e.g., @Profile or
@ConditionalOnProperty) and add proper access control (e.g., require an
admin/developer role via security annotations or explicit authentication checks)
so only authorized users can call it; ensure any logging or error messages do
not leak sensitive data and document removal/deactivation in the deployment
checklist.
| private Long getUserIdFromAuthentication(Authentication authentication) { | ||
| String currentUserEmail = authentication.getName(); | ||
| Optional<User> currentUserOpt = userService.findByEmail(currentUserEmail); | ||
|
|
||
| if (currentUserOpt.isEmpty()) { | ||
| throw new BusinessException(ChatErrorCode.USER_NOT_FOUND); | ||
| } | ||
|
|
||
| return currentUserOpt.get().getUserId(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Authentication null 방어 코드 추가
authentication 또는 authentication.getName()이 null일 경우 NPE가 발생합니다. 방어적 체크를 추가해 주세요.
- private Long getUserIdFromAuthentication(Authentication authentication) {
- String currentUserEmail = authentication.getName();
+ private Long getUserIdFromAuthentication(Authentication authentication) {
+ if (authentication == null || authentication.getName() == null) {
+ throw new BusinessException(ChatErrorCode.USER_NOT_FOUND);
+ }
+ String currentUserEmail = authentication.getName();
Optional<User> currentUserOpt = userService.findByEmail(currentUserEmail);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private Long getUserIdFromAuthentication(Authentication authentication) { | |
| String currentUserEmail = authentication.getName(); | |
| Optional<User> currentUserOpt = userService.findByEmail(currentUserEmail); | |
| if (currentUserOpt.isEmpty()) { | |
| throw new BusinessException(ChatErrorCode.USER_NOT_FOUND); | |
| } | |
| return currentUserOpt.get().getUserId(); | |
| } | |
| private Long getUserIdFromAuthentication(Authentication authentication) { | |
| if (authentication == null || authentication.getName() == null) { | |
| throw new BusinessException(ChatErrorCode.USER_NOT_FOUND); | |
| } | |
| String currentUserEmail = authentication.getName(); | |
| Optional<User> currentUserOpt = userService.findByEmail(currentUserEmail); | |
| if (currentUserOpt.isEmpty()) { | |
| throw new BusinessException(ChatErrorCode.USER_NOT_FOUND); | |
| } | |
| return currentUserOpt.get().getUserId(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java
around lines 37 to 46, add defensive null checks: verify authentication is not
null and authentication.getName() is not null/blank before calling userService;
if either is null/blank, throw new
BusinessException(ChatErrorCode.USER_NOT_FOUND) (or the appropriate auth-related
ChatErrorCode), otherwise retrieve the email and continue to lookup the user and
return userId.
| private final ContractMongoRepository repository; | ||
| private final RestTemplate restTemplate; | ||
| private final IdentityVerificationService identityVerificationService; | ||
|
|
||
| private final ContractMapper contractMapper; | ||
| private final TenantPreContractMapper tenantMapper; | ||
|
|
||
| @Value("${ai.server.url:http://localhost:8000}") | ||
| private String aiServerUrl; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
RestTemplate 타임아웃/빈 구성 확인 권장
외부 AI 서버 호출은 네트워크 문제로 장시간 블로킹될 수 있습니다. RestTemplate에 연결/읽기 타임아웃을 설정하고, 커넥션 풀링이 가능한 HttpComponentsClientHttpRequestFactory 등으로 Bean을 구성하는 것을 권장합니다. Spring 5 이상이라면 WebClient로 전환도 고려하세요.
[offer_architecture_advice]
레포지토리 내 RestTemplate Bean 구성 여부를 확인하는 스크립트입니다:
🏁 Script executed:
#!/bin/bash
# RestTemplate Bean 또는 타임아웃 설정 존재 여부 확인
rg -n -C3 '@Bean\s+public\s+RestTemplate|new\s+RestTemplate\s*\(' --type javaLength of output: 1187
RestTemplate 타임아웃 설정 및 빈 통합 필요
src/main/java/org/scoula/global/oauth2/config/OAuth2ManualConfig.java의restTemplate()메서드가 기본new RestTemplate()만 반환해 타임아웃·커넥션 풀 설정이 적용되지 않습니다.- 이미
src/main/java/org/scoula/global/config/RestTemplateConfig.java에서HttpComponentsClientHttpRequestFactory를 이용해 커넥션/리드 타임아웃(10s/30s)과 커넥션 풀링을 설정한 RestTemplate Bean을 정의하고 있으므로, OAuth2ManualConfig에서도 해당 Bean을 주입받아 사용하거나 동일한 factory를 적용하도록 통합하세요. - 선택 사항으로, Spring 5 이상을 사용 중이라면 WebClient로 전환해 논블로킹 HTTP 호출을 도입하는 방안도 고려해 보시기 바랍니다.
🤖 Prompt for AI Agents
In src/main/java/org/scoula/domain/contract/service/ContractFixService.java
around lines 28 to 36, the RestTemplate usage must rely on the application-wide
configured bean (with HttpComponentsClientHttpRequestFactory, connection pooling
and timeouts) instead of creating a plain new RestTemplate: update the
OAuth2ManualConfig to inject the RestTemplate bean from
src/main/java/org/scoula/global/config/RestTemplateConfig (or inject its
configured ClientHttpRequestFactory) and remove any direct new RestTemplate()
creation so the 10s/30s timeouts and pooling are applied; alternatively,
consider migrating to a configured WebClient for non-blocking calls if on Spring
5+.
| Long ownerContractId = contractMapper.getOwnerId(contractChatId); | ||
| Long buyerContractId = contractMapper.getBuyerId(contractChatId); | ||
| IdentityVerificationInfoVO ownerVO = | ||
| identityVerificationService.getDecryptedVerificationInfo( | ||
| contractChatId, ownerContractId); | ||
| IdentityVerificationInfoVO buyerVO = | ||
| identityVerificationService.getDecryptedVerificationInfo( | ||
| contractChatId, buyerContractId); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
소유자/임차인 식별자 null 가능성 검토
contractMapper.getOwnerId/getBuyerId가 null을 반환할 가능성이 있다면, 이후 getDecryptedVerificationInfo 호출에서 NPE나 비의도적 동작이 발생합니다. 매퍼가 non-null을 보장하는지 확인하고, 필요 시 방어적 검사를 추가하세요.
null 가능성을 점검하기 위한 검색 스크립트입니다(매퍼 정의/SQL을 확인):
🏁 Script executed:
#!/bin/bash
# ownerId/buyerId 매퍼 반환 보장 여부 확인(매퍼 및 SQL 파일에서 null 가능성 탐색)
rg -n -C2 'interface\s+ContractMapper|class\s+ContractMapper' --type java
rg -n -C2 'getOwnerId\s*\(|getBuyerId\s*\(' --type java
rg -n -C2 'getOwnerId|getBuyerId' -g '!**/build/**'Length of output: 113464
매퍼 XML에서 getOwnerId/getBuyerId의 WHERE 절 및 파라미터 바인딩을 확인하기 위해 아래 스크립트를 실행해주세요.
🏁 Script executed:
#!/bin/bash
# ContractMapper.xml에서 getOwnerId/getBuyerId SQL 매핑 전체 확인
sed -n '1,50p' src/main/resources/org/scoula/domain/contract/mapper/ContractMapper.xmlLength of output: 2094
방어적 null 검사 추가 필요: getOwnerId/getBuyerId는 null 반환 가능
ContractMapper XML을 확인한 결과, getOwnerId 및 getBuyerId SQL 매핑은
- 행이 없거나
owner_id/buyer_id컬럼이 null인 경우
null을 반환합니다. MyBatis는 이를 예외 없이 null로 매핑하기 때문에, 이후
identityVerificationService.getDecryptedVerificationInfo(...) 호출 시 NPE가 발생할 수 있습니다.
조치 사항:
- ContractFixService.java (라인 49–57)에서
ownerContractId,buyerContractId의 null 여부를 검사 - null 일 때 적절한 예외(BusinessException 등) 또는 early return 처리 추가
- 동일 패턴을 사용하는 ContractServiceImpl에도 동일 방어 로직 적용
대체 예시:
@@ src/main/java/org/scoula/domain/contract/service/ContractFixService.java:47
- Long ownerContractId = contractMapper.getOwnerId(contractChatId);
- Long buyerContractId = contractMapper.getBuyerId(contractChatId);
+ Long ownerContractId = contractMapper.getOwnerId(contractChatId);
+ Long buyerContractId = contractMapper.getBuyerId(contractChatId);
+ if (ownerContractId == null || buyerContractId == null) {
+ throw new BusinessException(ContractErrorCode.CONTRACT_CHAT_NOT_FOUND,
+ String.format("유효하지 않은 계약 채팅 ID: %d (ownerId=%s, buyerId=%s)",
+ contractChatId, ownerContractId, buyerContractId));
+ }필요한 곳:
- src/main/java/org/scoula/domain/contract/service/ContractFixService.java:49–57
- src/main/java/org/scoula/domain/contract/service/ContractServiceImpl.java:113–117
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Long ownerContractId = contractMapper.getOwnerId(contractChatId); | |
| Long buyerContractId = contractMapper.getBuyerId(contractChatId); | |
| IdentityVerificationInfoVO ownerVO = | |
| identityVerificationService.getDecryptedVerificationInfo( | |
| contractChatId, ownerContractId); | |
| IdentityVerificationInfoVO buyerVO = | |
| identityVerificationService.getDecryptedVerificationInfo( | |
| contractChatId, buyerContractId); | |
| Long ownerContractId = contractMapper.getOwnerId(contractChatId); | |
| Long buyerContractId = contractMapper.getBuyerId(contractChatId); | |
| if (ownerContractId == null || buyerContractId == null) { | |
| throw new BusinessException( | |
| ContractErrorCode.CONTRACT_CHAT_NOT_FOUND, | |
| String.format( | |
| "유효하지 않은 계약 채팅 ID: %d (ownerId=%s, buyerId=%s)", | |
| contractChatId, ownerContractId, buyerContractId | |
| ) | |
| ); | |
| } | |
| IdentityVerificationInfoVO ownerVO = | |
| identityVerificationService.getDecryptedVerificationInfo( | |
| contractChatId, ownerContractId); | |
| IdentityVerificationInfoVO buyerVO = | |
| identityVerificationService.getDecryptedVerificationInfo( | |
| contractChatId, buyerContractId); |
🤖 Prompt for AI Agents
In src/main/java/org/scoula/domain/contract/service/ContractFixService.java
around lines 49–57, ownerContractId and buyerContractId returned from
contractMapper.getOwnerId/getBuyerId can be null; add defensive null checks
immediately after those calls and if either is null throw an appropriate
BusinessException (or perform an early return) with a clear message including
the contractChatId so you do not call identityVerificationService with a null
id; apply the same null-check-and-exception pattern to
src/main/java/org/scoula/domain/contract/service/ContractServiceImpl around
lines 113–117.
| @Override | ||
| public void configureClientInboundChannel(ChannelRegistration registration) { | ||
| registration.interceptors( | ||
| new ChannelInterceptor() { | ||
| @Override | ||
| public Message<?> preSend(Message<?> message, MessageChannel channel) { | ||
| StompHeaderAccessor accessor = | ||
| MessageHeaderAccessor.getAccessor( | ||
| message, StompHeaderAccessor.class); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
NPE 방지: StompHeaderAccessor가 null일 수 있음
MessageHeaderAccessor.getAccessor(...)가 null을 반환할 수 있으므로 방어 로직이 필요합니다.
- StompHeaderAccessor accessor =
- MessageHeaderAccessor.getAccessor(
- message, StompHeaderAccessor.class);
+ StompHeaderAccessor accessor =
+ MessageHeaderAccessor.getAccessor(
+ message, StompHeaderAccessor.class);
+ if (accessor == null) {
+ return message; // 헤더 접근 불가 시 그대로 통과
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| public void configureClientInboundChannel(ChannelRegistration registration) { | |
| registration.interceptors( | |
| new ChannelInterceptor() { | |
| @Override | |
| public Message<?> preSend(Message<?> message, MessageChannel channel) { | |
| StompHeaderAccessor accessor = | |
| MessageHeaderAccessor.getAccessor( | |
| message, StompHeaderAccessor.class); | |
| @Override | |
| public void configureClientInboundChannel(ChannelRegistration registration) { | |
| registration.interceptors( | |
| new ChannelInterceptor() { | |
| @Override | |
| public Message<?> preSend(Message<?> message, MessageChannel channel) { | |
| StompHeaderAccessor accessor = | |
| MessageHeaderAccessor.getAccessor( | |
| message, StompHeaderAccessor.class); | |
| if (accessor == null) { | |
| return message; // 헤더 접근 불가 시 그대로 통과 | |
| } | |
| // … remaining logic … |
🤖 Prompt for AI Agents
In src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java around
lines 61 to 70, the call to MessageHeaderAccessor.getAccessor(...) can return
null causing an NPE; add a null-check after obtaining the StompHeaderAccessor
and handle the null case (e.g., return the original message immediately or skip
header-dependent logic) so subsequent calls on accessor are guarded; implement
the null guard and keep existing behavior when accessor is present.
| if (authHeader != null && authHeader.startsWith("Bearer ")) { | ||
| try { | ||
| String token = authHeader.substring(7); | ||
| // 간단한 JWT 파싱 (실제로는 JwtUtil 사용 권장) | ||
| String[] parts = token.split("\\."); | ||
| if (parts.length == 3) { | ||
| String payload = | ||
| new String( | ||
| java.util.Base64.getDecoder() | ||
| .decode(parts[1])); | ||
| log.info("🔍 JWT 페이로드: {}", payload); | ||
|
|
||
| // 페이로드에서 sub (사용자 이메일) 추출 | ||
| if (payload.contains("\"sub\"")) { | ||
| String[] subParts = payload.split("\"sub\":\""); | ||
| if (subParts.length > 1) { | ||
| String userEmail = subParts[1].split("\"")[0]; | ||
| log.info("🔍 JWT에서 추출한 사용자: {}", userEmail); | ||
|
|
||
| // Principal 설정 | ||
| UsernamePasswordAuthenticationToken auth = | ||
| new UsernamePasswordAuthenticationToken( | ||
| userEmail, | ||
| null, | ||
| Collections.emptyList()); | ||
| accessor.setUser(auth); | ||
| log.info( | ||
| "✅ WebSocket 인증 성공 (JWT) - User: {}", | ||
| userEmail); | ||
| } | ||
| } | ||
| } | ||
| } catch (Exception e) { | ||
| log.error("❌ JWT 토큰 파싱 실패: {}", e.getMessage()); | ||
| } |
There was a problem hiding this comment.
치명적: 서명 검증 없이 JWT 페이로드만 디코딩하여 인증 토큰을 생성합니다
현재 구현은 단순 Base64 디코딩 + 문자열 파싱으로 sub를 추출하고, 그 값을 Principal로 설정합니다. 이 방식은 서명 검증이 전혀 없어 임의로 조작된 토큰으로 인증 우회가 가능합니다. 또한 3-인자 생성자를 사용한 UsernamePasswordAuthenticationToken은 기본적으로 authenticated=true가 되어 더 위험합니다.
권장: Spring Security의 JwtDecoder로 서명/만료 검증을 수행하고, 검증 실패 시 CONNECT를 거부하세요. 페이로드 전체를 로그로 남기는 것도 피해야 합니다.
검증 기반 구현 예시 패치(이 블록 내 대체):
- if (authHeader != null && authHeader.startsWith("Bearer ")) {
- try {
- String token = authHeader.substring(7);
- // 간단한 JWT 파싱 (실제로는 JwtUtil 사용 권장)
- String[] parts = token.split("\\.");
- if (parts.length == 3) {
- String payload =
- new String(
- java.util.Base64.getDecoder()
- .decode(parts[1]));
- log.info("🔍 JWT 페이로드: {}", payload);
-
- // 페이로드에서 sub (사용자 이메일) 추출
- if (payload.contains("\"sub\"")) {
- String[] subParts = payload.split("\"sub\":\"");
- if (subParts.length > 1) {
- String userEmail = subParts[1].split("\"")[0];
- log.info("🔍 JWT에서 추출한 사용자: {}", userEmail);
-
- // Principal 설정
- UsernamePasswordAuthenticationToken auth =
- new UsernamePasswordAuthenticationToken(
- userEmail,
- null,
- Collections.emptyList());
- accessor.setUser(auth);
- log.info(
- "✅ WebSocket 인증 성공 (JWT) - User: {}",
- userEmail);
- }
- }
- }
- } catch (Exception e) {
- log.error("❌ JWT 토큰 파싱 실패: {}", e.getMessage());
- }
- }
+ if (authHeader != null && authHeader.startsWith("Bearer ")) {
+ try {
+ String token = authHeader.substring(7);
+ // JwtDecoder를 통한 유효성 검증 및 클레임 파싱
+ org.springframework.security.oauth2.jwt.Jwt jwt =
+ jwtDecoder.decode(token);
+ String userEmail = jwt.getSubject();
+ if (userEmail != null && !userEmail.isEmpty()) {
+ UsernamePasswordAuthenticationToken auth =
+ new UsernamePasswordAuthenticationToken(
+ userEmail, null, Collections.emptyList());
+ accessor.setUser(auth);
+ if (log.isDebugEnabled()) {
+ log.debug("WebSocket 인증 성공 (JWT) - sub: {}", userEmail);
+ }
+ } else {
+ log.warn("JWT에 sub 클레임이 없습니다.");
+ }
+ } catch (org.springframework.security.oauth2.jwt.JwtException e) {
+ log.warn("JWT 유효성 검증 실패: {}", e.getMessage());
+ }
+ }위 패치를 사용하려면 클래스 수준에 JwtDecoder 주입이 필요합니다(아래 추가 코드 참고). 기존에 사내 JwtUtil이 있다면 동일 위치에서 대체 사용해도 됩니다.
클래스 외 추가/변경(참고):
// import 추가
import lombok.RequiredArgsConstructor;
import org.springframework.security.oauth2.jwt.JwtDecoder;
// 클래스 어노테이션
@RequiredArgsConstructor
// 필드 추가
private final JwtDecoder jwtDecoder;추가로, Base64 디코딩 시 문자셋 명시(StandardCharsets.UTF_8) 및 민감 정보 로그 제거를 권장합니다.
src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java
Outdated
Show resolved
Hide resolved
| // X-User-Id 헤더가 있으면 사용 (백업) | ||
| if (accessor.getUser() == null && userId != null && !userId.isEmpty()) { | ||
| UsernamePasswordAuthenticationToken auth = | ||
| new UsernamePasswordAuthenticationToken( | ||
| userId, null, Collections.emptyList()); | ||
| accessor.setUser(auth); | ||
| log.info("✅ WebSocket 인증 성공 (User-Id) - UserId: {}", userId); | ||
| } |
There was a problem hiding this comment.
치명적: 클라이언트 제어 가능한 X-User-Id 헤더를 인증 백업 경로로 사용
게이트웨이에서 강제로 주입·서명·정제되는 헤더가 아닌 이상, 클라이언트가 임의 사용자로 가장할 수 있습니다. 이 경로는 제거하거나, 신뢰 가능한 프록시에서 추가했음을 검증할 수 있을 때만 사용하세요(예: mTLS, 내부 네트워크, 서명된 헤더).
불가피하게 유지한다면 최소한 환경 변수/프로파일로 비활성화 가능하도록 feature flag를 두세요. 제거 패치 예시는 다음과 같습니다:
- // X-User-Id 헤더가 있으면 사용 (백업)
- if (accessor.getUser() == null && userId != null && !userId.isEmpty()) {
- UsernamePasswordAuthenticationToken auth =
- new UsernamePasswordAuthenticationToken(
- userId, null, Collections.emptyList());
- accessor.setUser(auth);
- log.info("✅ WebSocket 인증 성공 (User-Id) - UserId: {}", userId);
- }
+ // 보안상의 이유로 X-User-Id 백업 인증은 허용하지 않습니다.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // X-User-Id 헤더가 있으면 사용 (백업) | |
| if (accessor.getUser() == null && userId != null && !userId.isEmpty()) { | |
| UsernamePasswordAuthenticationToken auth = | |
| new UsernamePasswordAuthenticationToken( | |
| userId, null, Collections.emptyList()); | |
| accessor.setUser(auth); | |
| log.info("✅ WebSocket 인증 성공 (User-Id) - UserId: {}", userId); | |
| } | |
| // 보안상의 이유로 X-User-Id 백업 인증은 허용하지 않습니다. |
🤖 Prompt for AI Agents
In src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java around
lines 121 to 128, the code accepts an untrusted X-User-Id header as a backup
authentication vector which allows client impersonation; remove this fallback or
gate it behind a clear, configurable trust check and a feature flag.
Specifically, either delete the block that sets accessor.setUser(...) from
X-User-Id, or wrap it so it only runs when (a) a runtime feature flag
(env/profile property) enabling header-based auth is true, and (b) the request
is verified to come from a trusted source (e.g., check a trusted-proxy list,
validate the header was injected by the gateway via mTLS or a signed header, or
verify X-Forwarded-For/trust proxy metadata). Add the feature flag to
application properties (disabled by default) and log a WARN when the unsafe
fallback is enabled.
| return message; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
미인증 CONNECT 차단 로직 추가 권장
CONNECT 처리 후에도 Principal이 없으면 메시지를 그대로 통과시키고 있습니다. 인증 실패 시 null을 반환해 CONNECT를 거부하는 방어 로직을 추가하세요.
- return message;
+ if (StompCommand.CONNECT.equals(accessor.getCommand())
+ && accessor.getUser() == null) {
+ log.warn("WebSocket CONNECT 인증 실패: Principal이 설정되지 않았습니다. 연결 차단");
+ return null; // CONNECT 차단
+ }
+ return message;🤖 Prompt for AI Agents
In src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java around
lines 137-138, after handling CONNECT you currently return the message even when
the Principal is null; change this to reject unauthenticated CONNECTs by
returning null when the incoming STOMP command is CONNECT (or CONNECT/WS
equivalent) and the Principal is missing. Implement a check that if
(stompCommand == CONNECT && principal == null) return null; otherwise continue
returning the message, so CONNECT frames without authentication are blocked.
| sanitizedLegalityResponse = String.valueOf(legalityResponse); | ||
| } | ||
| sanitizedLegalityResponse = sanitizedLegalityResponse.replaceAll("[\\r\\n]", " "); | ||
| log.info("적법성 검사 응답: {}", sanitizedLegalityResponse); |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fully mitigate log injection, all control characters (not just newlines and carriage returns) should be removed or replaced from user-controlled input before logging. The best way to do this is to use a regular expression to replace all characters in the Unicode control character class (\p{Cntrl}) with a space or another safe character. Additionally, user input should be clearly marked in the log entry, for example by wrapping it in single quotes or another delimiter. The change should be made in src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java at the point where sanitizedLegalityResponse is prepared for logging (line 2501). No new imports are needed, as the code already uses ObjectMapper and standard Java classes.
| @@ -2498,8 +2498,9 @@ | ||
| } catch (Exception ex) { | ||
| sanitizedLegalityResponse = String.valueOf(legalityResponse); | ||
| } | ||
| sanitizedLegalityResponse = sanitizedLegalityResponse.replaceAll("[\\r\\n]", " "); | ||
| log.info("적법성 검사 응답: {}", sanitizedLegalityResponse); | ||
| // Remove all control characters to prevent log injection | ||
| sanitizedLegalityResponse = sanitizedLegalityResponse.replaceAll("[\\p{Cntrl}]", " "); | ||
| log.info("적법성 검사 응답: '{}'", sanitizedLegalityResponse); | ||
| if (legalityResponse instanceof LegalityDTO) { | ||
| LegalityDTO legalityDTO = (LegalityDTO) legalityResponse; | ||
| log.info("LegalityDTO로 응답 파싱 성공"); |
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java
Fixed
Show fixed
Hide fixed
| AiMessageLegal(contractChatId, violationMessage.toString()); | ||
|
|
||
| try { | ||
| Thread.sleep(2000); |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fully address the log injection risk, all control characters (not just newlines and carriage returns) should be removed or replaced from user-controlled input before logging. The best way to do this is to use a regular expression that matches all control characters (e.g., [\p{Cntrl}]) and replace them with a space or another safe character. This should be applied to the sanitizedMessage variable before it is logged. The change should be made in src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java, specifically in the block where sanitizedMessage is created and logged. No new dependencies are required, as Java's String.replaceAll and regex support are sufficient.
| @@ -2564,7 +2564,7 @@ | ||
| "✅ 개선 방안\n %s\n", violation.getImprovementExample())); | ||
| } | ||
| String sanitizedMessage = | ||
| violationMessage.toString().replaceAll("[\\r\\n]", " "); | ||
| violationMessage.toString().replaceAll("[\\p{Cntrl}]", " "); | ||
| log.info("전송할 메시지: {}", sanitizedMessage); | ||
| AiMessageLegal(contractChatId, violationMessage.toString()); | ||
|
|
| } | ||
| // Remove newlines and carriage returns | ||
| resStr = resStr.replaceAll("[\\r\\n]", " "); | ||
| log.warn("AI 응답 값 확인: {}", resStr); |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the log injection vulnerability, we should sanitize the string being logged by removing all control characters, not just newlines and carriage returns. This can be done using a regular expression that matches all control characters ([\p{Cntrl}]). The best way to do this is to update the sanitization step for resStr on line 83 to use .replaceAll("[\\p{Cntrl}]", " ") instead of just \r and \n. This change should be made in the block where resStr is prepared for logging, just before the log statement on line 84. No new imports are needed, as the required methods are already available in the JDK.
| @@ -79,8 +79,8 @@ | ||
| } catch (Exception ex) { | ||
| resStr = res.toString(); | ||
| } | ||
| // Remove newlines and carriage returns | ||
| resStr = resStr.replaceAll("[\\r\\n]", " "); | ||
| // Remove all control characters to prevent log injection | ||
| resStr = resStr.replaceAll("[\\p{Cntrl}]", " "); | ||
| log.warn("AI 응답 값 확인: {}", resStr); | ||
| log.warn("AI 응답 헤더 확인: {}", response.getStatusCode()); | ||
| if (response.getStatusCode().is2xxSuccessful() && response.getBody() != null) { |
| } | ||
| // Remove newlines and carriage returns | ||
| responseBodyStr = responseBodyStr.replaceAll("[\\p{Cntrl}]", " "); | ||
| log.error("AI server error response (sanitized): {}", responseBodyStr); |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the log injection vulnerability, we should ensure that any data derived from untrusted sources (such as HTTP responses) is sanitized before being logged. The best way to do this is to remove all newline (\n) and carriage return (\r) characters from the string before logging, as these can be used to forge new log entries. Additionally, we should clearly mark user input in the log entry to prevent confusion. In this case, we should update the sanitization on line 98 to explicitly remove \r and \n (using .replaceAll("[\\r\\n]", " ")), and optionally escape other control characters if desired. The log entry on line 99 should remain unchanged except for the improved sanitization.
Required changes:
- In
src/main/java/org/scoula/domain/contract/service/ContractFixService.java, update the sanitization ofresponseBodyStrbefore logging on line 98 to remove newlines and carriage returns using.replaceAll("[\\r\\n]", " "). - No new imports or methods are required, as the code already uses
ObjectMapperand standard Java string methods.
| @@ -95,7 +95,7 @@ | ||
| responseBodyStr = String.valueOf(response.getBody()); | ||
| } | ||
| // Remove newlines and carriage returns | ||
| responseBodyStr = responseBodyStr.replaceAll("[\\p{Cntrl}]", " "); | ||
| responseBodyStr = responseBodyStr.replaceAll("[\\r\\n]", " "); | ||
| log.error("AI server error response (sanitized): {}", responseBodyStr); | ||
| throw new BusinessException(ContractException.CONTRACT_AI_SERVER_ERROR); | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (3)
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (3)
2809-2813: URL 조립 시 null 파라미터 결합 버그getContractChatStatus가 null을 반환할 수 있어 "…null"이 URL에 붙을 수 있습니다. 빈 문자열로 대체하세요.
- return baseUrl + contractChatUrl + contractChatRoomId.toString() + param; + return baseUrl + contractChatUrl + contractChatRoomId + (param != null ? param : "");또한 STEP4 상태에 대한 쿼리 파라미터 매핑이 없다면 아래 코멘트의 수정과 함께 일관성을 맞춰주세요.
2393-2397: 수정요청 보류 상태 Redis 키 불일치 — 조회 실패 버그requestFinalContractModification/getPendingModificationRequest는
- key: final-contract:modification:{contractChatId}:{ownerId}
형식을 쓰지만, hasPendingModificationRequest는- key: final-contract:modification:{contractChatId}:{clauseOrder}
를 조회합니다. 키가 달라 항상 false가 나올 수 있습니다. ownerId 기반으로 맞추세요.- public boolean hasPendingModificationRequest(Long contractChatId, Integer clauseOrder) { - String redisKey = "final-contract:modification:" + contractChatId + ":" + clauseOrder; - return stringRedisTemplate.hasKey(redisKey); - } + public boolean hasPendingModificationRequest(Long contractChatId, Integer clauseOrder) { + ContractChat contractChat = contractChatMapper.findByContractChatId(contractChatId); + if (contractChat == null) { + return false; + } + String redisKey = "final-contract:modification:" + contractChatId + ":" + contractChat.getOwnerId(); + // 선택적으로 clauseOrder 일치 여부까지 확인하려면 값(JSON) 파싱 로직을 getPending과 동일하게 재사용하세요. + return stringRedisTemplate.hasKey(redisKey); + }
2710-2756: 최종 특약 삭제 수락 키 불일치 — 요청 키와 수락 키가 달라 조회 실패requestFinalContractDeletion는
- key: final-contract:deletion:{contractChatId}:{ownerId}
형식에 clauseOrder를 값으로 저장합니다. 그러나 acceptFinalContractDeletion는- key: final-contract:deletion:{contractChatId}:{clauseOrder}
를 조회하고 있어 매칭되지 않습니다. ownerId를 사용하도록 수정하세요.- String redisKey = "final-contract:deletion:" + contractChatId + ":" + clauseOrder; + String redisKey = "final-contract:deletion:" + contractChatId + ":" + contractChat.getOwnerId();이후 storedOwnerId 검증은 동일하게 동작합니다.
♻️ Duplicate comments (5)
src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java (3)
67-70: NPE 위험: StompHeaderAccessor null 가능 — 즉시 가드 추가 필요
MessageHeaderAccessor.getAccessor(...)는 null을 반환할 수 있습니다. 현재 코드에서는 바로accessor.getCommand()를 호출하므로 NPE 가능성이 있습니다. 아래처럼 null 가드를 추가해주세요.StompHeaderAccessor accessor = MessageHeaderAccessor.getAccessor( message, StompHeaderAccessor.class); + if (accessor == null) { + // 헤더 접근 불가 시 그대로 통과 + return message; + }
132-133: 미인증 CONNECT를 허용 — Principal 없으면 연결을 차단해야 함CONNECT 처리 후에도
accessor.getUser()가 null이면 연결을 거부해야 합니다. 그렇지 않으면 인증 없이 STOMP 세션이 열릴 수 있습니다.- return message; + if (StompCommand.CONNECT.equals(accessor.getCommand()) + && accessor.getUser() == null) { + log.warn("WebSocket CONNECT 인증 실패: Principal이 설정되지 않았습니다. 연결 차단"); + return null; // CONNECT 거부 + } + return message;
117-123: 클라이언트 제어 가능한 X-User-Id 헤더를 인증 백업 경로로 사용 — 사용자 가장 취약점게이트웨이에서 신뢰 가능한 방식으로 서명/주입되지 않는 이상, 임의 사용자로 가장이 가능합니다. 본 경로는 제거하거나 강력한 신뢰 검증 + 기능 플래그로 제한하세요. 최소한 기본값은 비활성화가 안전합니다.
즉시 제거 패치(권장):
- // X-User-Id 헤더가 있으면 사용 (백업) - if (accessor.getUser() == null && userId != null && !userId.isEmpty()) { - UsernamePasswordAuthenticationToken auth = - new UsernamePasswordAuthenticationToken( - userId, null, Collections.emptyList()); - accessor.setUser(auth); - }src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (1)
2475-2615: 적법성 검사 결과 로깅의 로그 인젝션 위험 — 제어문자 완전 제거 및 길이 제한 권장현재 \r,\n 만 제거합니다. CodeQL 경고대로 제어문자 전체를 제거하고 길이를 제한하세요. 또한 메서드 필드 objectMapper를 재사용하세요.
- Object legalityResponse = contractFixService.getLegality(contractChatId, buyerId); - String sanitizedLegalityResponse; - try { - ObjectMapper objectMapper = new ObjectMapper(); - sanitizedLegalityResponse = objectMapper.writeValueAsString(legalityResponse); - } catch (Exception ex) { - sanitizedLegalityResponse = String.valueOf(legalityResponse); - } - sanitizedLegalityResponse = sanitizedLegalityResponse.replaceAll("[\\r\\n]", " "); - log.info("적법성 검사 응답: {}", sanitizedLegalityResponse); + Object legalityResponse = contractFixService.getLegality(contractChatId, buyerId); + String sanitizedLegalityResponse; + try { + sanitizedLegalityResponse = this.objectMapper.writeValueAsString(legalityResponse); + } catch (Exception ex) { + sanitizedLegalityResponse = String.valueOf(legalityResponse); + } + // 모든 제어문자 제거 + 길이 제한 + sanitizedLegalityResponse = sanitizedLegalityResponse.replaceAll("[\\p{Cntrl}]", " "); + if (sanitizedLegalityResponse.length() > 2000) { + sanitizedLegalityResponse = sanitizedLegalityResponse.substring(0, 2000) + "..."; + } + log.info("적법성 검사 응답(요약): {}", sanitizedLegalityResponse); ... - String sanitizedViolation = violation == null ? "null" : violation.toString().replaceAll("[\\r\\n]", " "); - log.info("위반 사항 {}: {}", i + 1, sanitizedViolation); + String sanitizedViolation; + try { + sanitizedViolation = violation == null ? "null" : this.objectMapper.writeValueAsString(violation); + } catch (Exception ex) { + sanitizedViolation = String.valueOf(violation); + } + sanitizedViolation = sanitizedViolation.replaceAll("[\\p{Cntrl}]", " "); + if (sanitizedViolation.length() > 1000) { + sanitizedViolation = sanitizedViolation.substring(0, 1000) + "..."; + } + log.info("위반 사항 {}(요약): {}", i + 1, sanitizedViolation); ... - String sanitizedMessage = violationMessage.toString().replaceAll("[\\r\\n]", " "); + String sanitizedMessage = violationMessage.toString().replaceAll("[\\p{Cntrl}]", " "); log.info("전송할 메시지: {}", sanitizedMessage);src/main/java/org/scoula/domain/contract/service/ContractFixService.java (1)
49-56: owner/buyer 식별자 null 방어 로직 추가 필요 (이전 리뷰 동일 지적).
contractMapper.getOwnerId/getBuyerId가 null을 반환하면 이후getDecryptedVerificationInfo호출에서 NPE 또는 예기치 않은 동작이 발생할 수 있습니다. 호출 직후 null 체크로 방어해 주세요.아래와 같이 보완을 제안드립니다:
Long ownerContractId = contractMapper.getOwnerId(contractChatId); Long buyerContractId = contractMapper.getBuyerId(contractChatId); +if (ownerContractId == null || buyerContractId == null) { + log.warn("계약 참여자 식별자 조회 실패: contractChatId={}, ownerId={}, buyerId={}", + contractChatId, ownerContractId, buyerContractId); + throw new BusinessException(ContractException.CONTRACT_GET); +} IdentityVerificationInfoVO ownerVO = identityVerificationService.getDecryptedVerificationInfo( contractChatId, ownerContractId); IdentityVerificationInfoVO buyerVO = identityVerificationService.getDecryptedVerificationInfo( contractChatId, buyerContractId);
🧹 Nitpick comments (15)
src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java (3)
26-33: System.err 직접 출력 제거 — 프레임워크 로거 사용 또는 삭제프로덕션 코드에서 콘솔 출력은 노이즈와 운영 가시성 저하를 유발합니다. 로거(DEBUG)로 대체하거나 삭제하세요. 생성자/정적 블록의 출력은 특히 불필요합니다.
- static { - System.err.println("🚨🚨🚨 WebSocketConfig 클래스 로딩됨!"); - } - - public WebSocketConfig() { - System.err.println("🚨🚨🚨 WebSocketConfig 생성자 호출됨!"); - } + // 불필요한 콘솔 출력 제거
35-41: MessageBroker 설정 로그도 로거로 일원화System.err 대신 로거(DEBUG) 사용을 권장합니다.
- System.err.println("🚨🚨🚨 MessageBroker 설정 시작"); + if (log.isDebugEnabled()) log.debug("MessageBroker 설정 시작"); config.enableSimpleBroker("/topic", "/queue"); config.setApplicationDestinationPrefixes("/app"); config.setUserDestinationPrefix("/user"); - System.err.println("🚨🚨🚨 MessageBroker 설정 완료"); + if (log.isDebugEnabled()) log.debug("MessageBroker 설정 완료");
45-59: STOMP 엔드포인트 등록 로그/설정 개선: 로거 사용 및 Origin 화이트리스트 외부화 권장
- System.err 출력은 로거(DEBUG)로 변경하세요.
- Origin 화이트리스트를 하드코딩하지 말고 설정 파일/프로파일로 외부화하면 운영 변경이 용이합니다.
- System.err.println("🚨🚨🚨 STOMP 엔드포인트 등록 시작"); + if (log.isDebugEnabled()) log.debug("STOMP 엔드포인트 등록 시작"); // Nginx가 CORS 헤더를 추가하지만, Spring WebSocket도 Origin 검증이 필요 // setAllowedOriginPatterns를 사용하여 Spring의 Origin 검증은 허용하되 // 실제 CORS 헤더는 Nginx에서 관리 registry.addEndpoint("/ws") .setAllowedOriginPatterns( "http://localhost:5173", "https://localhost:5173", "https://itzeep.ariogi.kr", "https://www.itzeep.ariogi.kr", "http://itzeep.ariogi.kr", "http://www.itzeep.ariogi.kr") .withSockJS(); - System.err.println("🚨🚨🚨 STOMP 엔드포인트 등록 완료"); + if (log.isDebugEnabled()) log.debug("STOMP 엔드포인트 등록 완료");추가 제안:
- allowed origins를
application.yml예:scoula.websocket.allowed-origins리스트로 두고@Value혹은ConfigurationProperties로 주입.- 추후 트래픽 증가 시 내장 simple broker 대신 외부 메시지 브로커(RabbitMQ/STOMP 등), heartbeat 설정 검토.
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (9)
203-219: AI 메시지 전송 메서드 중복(DRY 위반) — 단일 헬퍼로 통합 권장AiMessage/AiMessageNext/AiMessageBtn/AiMessageLegal이 거의 동일한 본문을 가집니다. senderId만 다르므로 단일 private 헬퍼(sendAiMessage(senderId, ...))로 통합하면 중복 제거와 유지보수성이 크게 개선됩니다. 또한 인터페이스에 선언되어 있다면 @OverRide 추가를 권장합니다.
- public void AiMessageLegal(Long contractChatId, String content) { - final Long ai = 9996L; - ContractChatDocument aiMessage = - ContractChatDocument.builder() - .contractChatId(contractChatId.toString()) - .senderId(ai) - .receiverId(null) - .content(content) - .sendTime(LocalDateTime.now().toString()) - .build(); - contractChatMessageRepository.saveMessage(aiMessage); - contractChatMapper.updateLastMessage(contractChatId, content); - messagingTemplate.convertAndSend("/topic/contract-chat/" + contractChatId, aiMessage); - } + public void AiMessageLegal(Long contractChatId, String content) { + sendAiMessage(contractChatId, 9996L, content); + }추가로 클래스 내부에 다음 헬퍼를 도입하면 다른 AI 메서드도 일괄 치환 가능합니다:
private void sendAiMessage(Long contractChatId, Long senderId, String content) { ContractChatDocument msg = ContractChatDocument.builder() .contractChatId(contractChatId.toString()) .senderId(senderId) .receiverId(null) .content(content) .sendTime(LocalDateTime.now().toString()) .build(); contractChatMessageRepository.saveMessage(msg); contractChatMapper.updateLastMessage(contractChatId, content); messagingTemplate.convertAndSend("/topic/contract-chat/" + contractChatId, msg); }
519-526: REDIS + 인메모리 동시 사용으로 인한 이중 소스 위험leaveContractChatRoom에서 Redis 세트 조작 후 setContractChatUserOffline(인메모리 맵)까지 호출합니다. 현재 온라인 판별은 Redis 기준이므로 인메모리 맵은 사실상 데드 데이터입니다. 일관성을 위해 인메모리 경로 제거 또는 완전한 Redis 전환을 권장합니다.
- setContractChatUserOffline(userId, contractChatId);
531-572: 온라인 상태 디버그 로그 레벨 및 민감정보 노출 최소화 권장현재 INFO 레벨로 방 상태와 결과 맵 전체를 출력합니다. 운영 환경에서는 DEBUG로 내리고, 값은 식별자만 제한적으로 출력하는 것이 안전합니다.
- log.info("=== getContractChatOnlineStatus(REDIS) 시작 ==="); + log.debug("=== getContractChatOnlineStatus(REDIS) 시작 ==="); ... - log.info( + log.debug( "Owner({}) 온라인: {}, Buyer({}) 온라인: {}, 둘 다 온라인: {}", ... - log.info("=== getContractChatOnlineStatus 완료: {} ===", result); + log.debug("=== getContractChatOnlineStatus 완료 ===");
574-590: 디버그 메서드의 로그 인젝션/민감정보 노출 위험 — DEBUG 레벨로 하향 및 길이 제한 제안사용자 목록을 그대로 INFO로 출력하면 과도한 노출이 될 수 있습니다. DEBUG로 내리고, 항목 수 등 요약 위주로 제한하세요.
- log.info("=== 현재 모든 온라인 사용자 상태(REDIS) ==="); + log.debug("=== 현재 모든 온라인 사용자 상태(REDIS) ==="); ... - log.info("Redis Key: {}, 온라인 사용자(문자열): {}", rKey, members); + log.debug("Redis Key: {}, 온라인 사용자 수: {}", rKey, members != null ? members.size() : 0); ... - log.info("계약 채팅방 {} 온라인 사용자(Long): {}", contractChatId, asLongs); + log.debug("계약 채팅방 {} 온라인 사용자 수(Long): {}", contractChatId, asLongs.size());
629-638: 인메모리 온라인 상태 맵 유지 필요성 재검토이제 온라인 판별은 Redis 기반입니다. setContractChatUserOnline/Offline 및 getContractChatKey는 미사용 또는 영향이 모호합니다. 혼선을 줄이기 위해 제거하거나 명확히 주석 처리하세요.
Also applies to: 642-652
2420-2428: 확정 요청 키에 TTL 부재 — 영구 잠김 위험확정 요청 키(final-contract:confirmation:...)에 TTL이 없어 실패/미응답 시 재요청이 영구 차단될 수 있습니다. 적절한 만료(예: 24h)를 부여하세요.
- stringRedisTemplate.opsForValue().set(key, value); + stringRedisTemplate.opsForValue().set(key, value, Duration.ofHours(24));
2855-2863: 최종 계약 확인 요청 키에 TTL 부재 — 영구 잠김 위험final-contract:request 키에도 TTL을 설정해 재시도/복구 가능성을 보장하세요.
- stringRedisTemplate.opsForValue().set(key, value); + stringRedisTemplate.opsForValue().set(key, value, Duration.ofHours(24));
2475-2491: 서비스 메서드(@transactional) 내 Thread.sleep 사용은 지양 — 비동기 처리 또는 클라이언트 사이드 지연 권장Thread.sleep은 톰캣 워커 스레드를 불필요하게 블로킹하고, 트랜잭션 지속시간을 늘려 리소스 고갈을 유발할 수 있습니다. 메시지 간 지연은 클라이언트에서 처리하거나, @Async/이벤트 발행 등 비동기 워커로 옮기세요.
- try { - Thread.sleep(2000); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } ... - try { - Thread.sleep(2000); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - }Also applies to: 2585-2589
29-29: 미사용/와일드카드 import 정리 권장 (Spotless 포맷 실패 원인 후보)이 클래스에서는 org.springframework.http.* 를 사용하지 않습니다. 와일드카드 import는 스타일 규칙을 위반할 수 있어 제거를 권장합니다.
-import org.springframework.http.*;src/main/java/org/scoula/domain/contract/service/ContractFixService.java (3)
103-106: 예외 로깅은 스택트레이스 포함으로 변경 권장.메시지 문자열만 남기면 원인 분석이 어렵습니다. 스택트레이스를 함께 로그에 남겨주세요.
- log.error(e.getMessage()); + log.error("AI 서버 호출 실패", e);
60-71: 외부 AI 서버 호출의 안정성(회복력) 보강 검토.네트워크/서버 이상 시 즉시 실패하는 현재 구조에, 제한적 재시도(backoff), 회로차단기(Resilience4j), 지연/실패율 모니터링 등을 도입하면 운영 안정성이 크게 향상됩니다. Accept 헤더(
application/json)를 명시하는 것도 호환성에 도움이 됩니다.
109-122: Void → void로 반환 타입 변경 권장
사이드 이펙트 전용 메서드는 박싱된Void대신 원시void를 사용해 NPE 위험을 제거하고 API 의도를 명확히 하는 것이 좋습니다.영향 범위:
- src/main/java/org/scoula/domain/contract/service/ContractFixServiceInterface.java (라인 7)
- src/main/java/org/scoula/domain/contract/service/ContractFixService.java (라인 110–122)
- src/main/java/org/scoula/domain/contract/controller/ContractController.java (라인 77–79)
- src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java (라인 136–137)
- src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (라인 2476)
예시 변경안:
// ContractFixServiceInterface.java - Void saveSpecialContract(Long contractChatId, Long userId); + void saveSpecialContract(Long contractChatId, Long userId); // ContractFixService.java - public Void saveSpecialContract(Long contractChatId, Long userId) { + public void saveSpecialContract(Long contractChatId, Long userId) { validateUserId(contractChatId, userId); try { repository.saveSpecialContract(contractChatId); } catch (Exception e) { log.error("특약사항 저장 실패 ❌", e); throw new BusinessException(ContractException.CONTRACT_INSERT, e); } - return null; }콜러 수정 예시:
- ControllerImpl:
contractFixService.saveSpecialContract(id, userId); return ApiResponse.success(null);- ContractController, ContractChatServiceImpl 등 호출부에서도 반환값 제거 및
void시그니처 반영 필요위치 확인 스크립트:
rg -n -C2 '\bsaveSpecialContract\s*\(' --type java
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java(9 hunks)src/main/java/org/scoula/domain/contract/service/ContractFixService.java(1 hunks)src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/org/scoula/**/service/*Service{Interface,Impl}.java
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use interface-implementation pattern for services:
*ServiceInterface.java(interface) and*ServiceImpl.java(implementation)
Files:
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java
🧬 Code Graph Analysis (1)
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (2)
src/main/java/org/scoula/global/common/exception/BusinessException.java (1)
BusinessException(4-21)src/main/java/org/scoula/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(4-13)
🪛 GitHub Check: CodeQL
src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java
[failure] 108-108: Insertion of sensitive information into log files
This potentially sensitive information is written to a log file.
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java
[failure] 2502-2502: Log Injection
This log entry depends on a user-provided value.
[failure] 2521-2521: Log Injection
This log entry depends on a user-provided value.
[failure] 2574-2574: Log Injection
This log entry depends on a user-provided value.
src/main/java/org/scoula/domain/contract/service/ContractFixService.java
[failure] 84-84: Log Injection
This log entry depends on a user-provided value.
[failure] 99-99: Log Injection
This log entry depends on a user-provided value.
🪛 GitHub Actions: CI
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java
[error] 2514-2514: Spotless Java format violations detected in ContractChatServiceImpl.java. Run './gradlew spotlessApply' to fix formatting.
🔇 Additional comments (4)
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (4)
107-126: 오프라인 차단 UX 의도 재확인 필요 및 재시도 전략 고려전송 차단 로직과 발신자 전용 에러 큐로의 피드백 설계는 합리적입니다. 다만, 메시지 영구 미저장 정책이 비즈니스 의도와 일치하는지 확인이 필요합니다. 필요 시 “대상 접속 시 재전송” 대기열(예: Redis ZSET with TTL)로 확장 가능합니다.
654-666: REDIS 기반 온라인 체크 LGTMRedis Set 멤버십을 통한 온라인 판별과 안전한 Boolean 처리(Boolean.TRUE.equals) 사용이 적절합니다.
2076-2094: STEP4 상태에 대한 URL 파라미터 매핑 누락 가능성acceptFinalContractConfirmation에서 상태를 STEP4로 업데이트합니다(라인 2467). getContractChatStatus는 STEP4 대응이 없어 null 반환 → URL 조립 시 "null"이 붙는 문제를 유발합니다. 프론트 요구사항에 맞춰 STEP4 매핑을 추가하세요.
case ROUND4: return "?step=3&round=4"; + case STEP4: + return "?step=4"; default: return null;필요 시 실제 기대 파라미터 값에 맞게 수정 바랍니다.
2514-2514: Spotless 포맷 실패 — 자동 정렬 적용 필요CI가 Spotless 포맷 위반을 보고했습니다. 로컬에서 './gradlew spotlessApply' 실행하여 정렬/공백/임포트 규칙을 일괄 적용해주세요.
| private final ContractMongoRepository repository; | ||
| private final RestTemplate restTemplate; | ||
| private final IdentityVerificationService identityVerificationService; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
RestTemplate Bean 주입은 적절합니다. 전역 설정(타임아웃/풀링) 적용되는지 확인만 해주세요.
직접 new RestTemplate() 생성 대신 빈 주입을 사용하고 있어 좋습니다. 앱 전역에서 타임아웃·커넥션 풀 설정된 빈과 동일한 인스턴스가 주입되는지 확인을 권장합니다.
다음 스크립트로 전역 RestTemplate 빈 정의와 직접 생성 사용 여부를 점검해보세요.
🏁 Script executed:
#!/bin/bash
# RestTemplate 빈 정의(설정 포함) 확인
rg -n -C2 '@Bean\s+public\s+RestTemplate|class\s+.*RestTemplateConfig' --type java
# 직접 new RestTemplate() 사용 위치가 남아있는지 확인
rg -n -C2 'new\s+RestTemplate\s*\(' --type javaLength of output: 1434
전역 RestTemplate 빈 설정 확인 및 중복 빈 제거 필요
OAuth2ManualConfig에서 new RestTemplate()을 사용해 별도 Bean이 생성되고 있어, 타임아웃·커넥션 풀 설정이 적용된 전역 빈 대신 해당 빈이 주입될 수 있습니다. 아래를 검토 및 수정해주세요.
- src/main/java/org/scoula/global/oauth2/config/OAuth2ManualConfig.java:18
return new RestTemplate();— 전역 설정 누락
- src/main/java/org/scoula/global/config/RestTemplateConfig.java:15–19
new RestTemplate(factory)로 타임아웃(30초) 설정된 빈 정의
조치 제안:
- OAuth2ManualConfig의 RestTemplate Bean을 제거하거나 전역 RestTemplateConfig에서 생성된 빈을 재사용하도록 통합
- 필요한 경우
@Qualifier또는 Bean 이름을 명시해 의도한 빈이 주입되도록 설정
🤖 Prompt for AI Agents
In src/main/java/org/scoula/domain/contract/service/ContractFixService.java
around lines 28–31 and related config files
(src/main/java/org/scoula/global/oauth2/config/OAuth2ManualConfig.java line ~18
and src/main/java/org/scoula/global/config/RestTemplateConfig.java lines 15–19),
a local RestTemplate created via new RestTemplate() in OAuth2ManualConfig can
override the globally-configured RestTemplate with timeouts/connection pool;
remove the redundant RestTemplate bean from OAuth2ManualConfig or change it to
inject the global bean instead, and if multiple RestTemplate beans must exist
mark the global one @Primary or use @Qualifier/@Bean name in both config and
ContractFixService so the timeout-configured RestTemplate from
RestTemplateConfig is always injected.
| ResponseEntity<LegalityDTO> response = | ||
| restTemplate.exchange(url, HttpMethod.POST, requestEntity, LegalityDTO.class); | ||
| LegalityDTO res = response.getBody(); | ||
| assert res != null; | ||
|
|
||
| String resStr; | ||
| try { | ||
| ObjectMapper objectMapper = new ObjectMapper(); | ||
| resStr = objectMapper.writeValueAsString(res); | ||
| } catch (Exception ex) { | ||
| resStr = res.toString(); | ||
| } | ||
| // Remove newlines and carriage returns | ||
| resStr = resStr.replaceAll("[\\r\\n]", " "); | ||
| log.warn("AI 응답 값 확인: {}", resStr); | ||
| log.warn("AI 응답 헤더 확인: {}", response.getStatusCode()); | ||
| if (response.getStatusCode().is2xxSuccessful() && response.getBody() != null) { | ||
| return response.getBody(); | ||
| } else { | ||
| // Sanitize response body before logging to prevent log injection | ||
| String responseBodyStr; | ||
| try { | ||
| ObjectMapper objectMapper = new ObjectMapper(); | ||
| responseBodyStr = objectMapper.writeValueAsString(response.getBody()); | ||
| } catch (Exception ex) { | ||
| responseBodyStr = String.valueOf(response.getBody()); | ||
| } | ||
| // Remove newlines and carriage returns | ||
| responseBodyStr = responseBodyStr.replaceAll("[\\p{Cntrl}]", " "); | ||
| log.error("AI server error response (sanitized): {}", responseBodyStr); | ||
| throw new BusinessException(ContractException.CONTRACT_AI_SERVER_ERROR); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
CodeQL(Log Injection) 경고 및 PII 노출 가능성: 응답 로깅 재구성 + assert 제거 + 보안적 로그 살균/축약 적용
현 로깅은 응답 본문을 warn 레벨로 그대로(개행/제어문자만 제거) 출력하고 있어 CodeQL이 지적한 로그 인젝션(라인 84, 99) 및 개인정보(PII) 노출 위험이 있습니다. 또한 assert res != null은 프로덕션에선 기본 비활성화되어 안전장치로 작동하지 않습니다.
- 비정상 응답 시에만 에러 로그(살균/길이 제한) 출력
- 정상 응답은 디버그로 축약 로깅(기본은 로그 미출력)
- 메시지 표현 수정(“헤더” → “상태 코드”)
- 명시적 null 검증으로 assert 제거
아래와 같이 변경을 제안드립니다.
- ResponseEntity<LegalityDTO> response =
- restTemplate.exchange(url, HttpMethod.POST, requestEntity, LegalityDTO.class);
- LegalityDTO res = response.getBody();
- assert res != null;
-
- String resStr;
- try {
- ObjectMapper objectMapper = new ObjectMapper();
- resStr = objectMapper.writeValueAsString(res);
- } catch (Exception ex) {
- resStr = res.toString();
- }
- // Remove newlines and carriage returns
- resStr = resStr.replaceAll("[\\r\\n]", " ");
- log.warn("AI 응답 값 확인: {}", resStr);
- log.warn("AI 응답 헤더 확인: {}", response.getStatusCode());
- if (response.getStatusCode().is2xxSuccessful() && response.getBody() != null) {
- return response.getBody();
- } else {
- // Sanitize response body before logging to prevent log injection
- String responseBodyStr;
- try {
- ObjectMapper objectMapper = new ObjectMapper();
- responseBodyStr = objectMapper.writeValueAsString(response.getBody());
- } catch (Exception ex) {
- responseBodyStr = String.valueOf(response.getBody());
- }
- // Remove newlines and carriage returns
- responseBodyStr = responseBodyStr.replaceAll("[\\p{Cntrl}]", " ");
- log.error("AI server error response (sanitized): {}", responseBodyStr);
- throw new BusinessException(ContractException.CONTRACT_AI_SERVER_ERROR);
- }
+ ResponseEntity<LegalityDTO> response =
+ restTemplate.exchange(url, HttpMethod.POST, requestEntity, LegalityDTO.class);
+ if (!response.getStatusCode().is2xxSuccessful() || response.getBody() == null) {
+ String responseBodyStr = sanitizeForLog(response.getBody(), 2048);
+ log.error("AI 서버 오류 응답: statusCode={}, body={}", response.getStatusCode(), responseBodyStr);
+ throw new BusinessException(ContractException.CONTRACT_AI_SERVER_ERROR);
+ }
+ LegalityDTO res = response.getBody();
+ if (log.isDebugEnabled()) {
+ String resStr = sanitizeForLog(res, 2048);
+ log.debug("AI 응답 본문(요약): {}", resStr);
+ log.debug("AI 응답 상태 코드: {}", response.getStatusCode());
+ }
+ return res;아래 보조 유틸리티를 클래스 내부에 추가해 주세요(로그 살균/축약 공통화).
추가 코드(선택 라인 외부):
// 클래스 내부에 추가
private static String sanitizeForLog(Object value, int maxLen) {
String s;
try {
s = new com.fasterxml.jackson.databind.ObjectMapper().writeValueAsString(value);
} catch (Exception ex) {
s = String.valueOf(value);
}
// 제어문자 제거 및 공백 정규화
s = s.replaceAll("[\\p{Cntrl}]", " ").replaceAll("\\s+", " ").trim();
if (s.length() > maxLen) {
s = s.substring(0, maxLen) + "...";
}
return s;
}🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 84-84: Log Injection
This log entry depends on a user-provided value.
[failure] 99-99: Log Injection
This log entry depends on a user-provided value.
🤖 Prompt for AI Agents
In src/main/java/org/scoula/domain/contract/service/ContractFixService.java
around lines 70-101, remove the assert and replace it with an explicit null
check for response.getBody(); move normal successful-response logging to DEBUG
and only log a truncated, sanitized payload on success (or skip logging
entirely), while on non-2xx responses log a sanitized and length-limited error
message at ERROR; add a private sanitizeForLog(Object value, int maxLen) helper
in the class that serializes to JSON (fallback to toString), strips control
characters and collapses whitespace, trims, and truncates to maxLen with an
ellipsis, then use that helper when logging response body and change the log
label from "헤더" to "상태 코드".
| log.info( | ||
| "🔍 받은 헤더 - Authorization: {}, X-User-Id: {}", | ||
| authHeader != null ? "있음" : "없음", | ||
| userId); |
There was a problem hiding this comment.
민감 정보 로그 제거 및 레벨 하향
X-User-Id 값을 그대로 INFO로 기록하면 PII/식별자 노출 위험이 있습니다. 값은 마스킹/존재 여부만 DEBUG로 기록하세요. CodeQL 경고(“Insertion of sensitive information into log files”)와도 일치합니다.
- log.info(
- "🔍 받은 헤더 - Authorization: {}, X-User-Id: {}",
- authHeader != null ? "있음" : "없음",
- userId);
+ if (log.isDebugEnabled()) {
+ log.debug(
+ "🔍 받은 헤더 - Authorization: {}, X-User-Id: {}",
+ authHeader != null ? "있음" : "없음",
+ (userId != null && !userId.isEmpty()) ? "있음" : "없음");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| log.info( | |
| "🔍 받은 헤더 - Authorization: {}, X-User-Id: {}", | |
| authHeader != null ? "있음" : "없음", | |
| userId); | |
| if (log.isDebugEnabled()) { | |
| log.debug( | |
| "🔍 받은 헤더 - Authorization: {}, X-User-Id: {}", | |
| authHeader != null ? "있음" : "없음", | |
| (userId != null && !userId.isEmpty()) ? "있음" : "없음"); | |
| } |
| // JWT 토큰에서 사용자 ID 추출 | ||
| if (authHeader != null && authHeader.startsWith("Bearer ")) { | ||
| try { | ||
| String token = authHeader.substring(7); | ||
| // 간단한 JWT 파싱 (실제로는 JwtUtil 사용 권장) | ||
| String[] parts = token.split("\\."); | ||
| if (parts.length == 3) { | ||
| String payload = | ||
| new String( | ||
| java.util.Base64.getDecoder() | ||
| .decode(parts[1])); | ||
| // 페이로드에서 sub (사용자 이메일) 추출 | ||
| if (payload.contains("\"sub\"")) { | ||
| String[] subParts = payload.split("\"sub\":\""); | ||
| if (subParts.length > 1) { | ||
| String userEmail = subParts[1].split("\"")[0]; | ||
| // Principal 설정 | ||
| UsernamePasswordAuthenticationToken auth = | ||
| new UsernamePasswordAuthenticationToken( | ||
| userEmail, | ||
| null, | ||
| Collections.emptyList()); | ||
| accessor.setUser(auth); | ||
| log.info( | ||
| "✅ WebSocket 인증 성공 (JWT) - User: {}", | ||
| userEmail); | ||
| } | ||
| } | ||
| } | ||
| } catch (Exception e) { | ||
| log.error("❌ JWT 토큰 파싱 실패: {}", e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
치명적: JWT 서명 검증 없이 페이로드만 디코딩 — 인증 우회 취약점
Base64 디코딩으로 sub를 파싱하는 방식은 누구나 조작 가능한 토큰으로 인증을 우회할 수 있습니다. 반드시 JwtDecoder 또는 사내 JwtUtil 등으로 서명·만료 검증을 수행해야 합니다. 또한 민감 정보(이메일 등)를 INFO 레벨로 로그하지 마세요.
아래와 같이 블록을 교체하는 것을 권장합니다(예: Spring JwtDecoder 사용). 프로젝트에 사내 JwtUtil이 있다면 해당 유틸을 사용해 같은 위치에서 대체 가능합니다.
- if (authHeader != null && authHeader.startsWith("Bearer ")) {
- try {
- String token = authHeader.substring(7);
- // 간단한 JWT 파싱 (실제로는 JwtUtil 사용 권장)
- String[] parts = token.split("\\.");
- if (parts.length == 3) {
- String payload =
- new String(
- java.util.Base64.getDecoder()
- .decode(parts[1]));
- // 페이로드에서 sub (사용자 이메일) 추출
- if (payload.contains("\"sub\"")) {
- String[] subParts = payload.split("\"sub\":\"");
- if (subParts.length > 1) {
- String userEmail = subParts[1].split("\"")[0];
- // Principal 설정
- UsernamePasswordAuthenticationToken auth =
- new UsernamePasswordAuthenticationToken(
- userEmail,
- null,
- Collections.emptyList());
- accessor.setUser(auth);
- log.info(
- "✅ WebSocket 인증 성공 (JWT) - User: {}",
- userEmail);
- }
- }
- }
- } catch (Exception e) {
- log.error("❌ JWT 토큰 파싱 실패: {}", e.getMessage());
- }
- }
+ if (authHeader != null && authHeader.startsWith("Bearer ")) {
+ try {
+ String token = authHeader.substring(7);
+ org.springframework.security.oauth2.jwt.Jwt jwt = jwtDecoder.decode(token);
+ String userEmail = jwt.getSubject();
+ if (userEmail != null && !userEmail.isEmpty()) {
+ UsernamePasswordAuthenticationToken auth =
+ new UsernamePasswordAuthenticationToken(
+ userEmail, null, Collections.emptyList());
+ accessor.setUser(auth);
+ if (log.isDebugEnabled()) {
+ log.debug("WebSocket 인증 성공 (JWT) - sub 존재");
+ }
+ } else {
+ log.warn("JWT에 sub 클레임이 없습니다.");
+ }
+ } catch (org.springframework.security.oauth2.jwt.JwtException e) {
+ log.warn("JWT 유효성 검증 실패: {}", e.getMessage());
+ }
+ }클래스 외 추가/변경(예시):
// import
import lombok.RequiredArgsConstructor;
import org.springframework.security.oauth2.jwt.JwtDecoder;
// 클래스 선언에 추가
@RequiredArgsConstructor
// 필드 추가
private final JwtDecoder jwtDecoder;주의:
JwtDecoder빈이 구성되어 있어야 합니다(예: JWK Set URI 기반NimbusJwtDecoder).- PII/클레임은 DEBUG 이하로만 제한해 로깅하세요(아래 별도 코멘트 참고).
원하시면 JwtDecoder 빈 구성 패치도 같이 드리겠습니다.
🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 108-108: Insertion of sensitive information into log files
This potentially sensitive information is written to a log file.
🤖 Prompt for AI Agents
In src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java around
lines 83 to 115, the code currently base64-decodes the JWT payload and trusts
the "sub" claim without signature or expiry verification and logs PII at INFO;
replace this block to validate the token using a proper JwtDecoder or your
project's JwtUtil (inject as a final field, add @RequiredArgsConstructor to the
class), call decoder.decode(token) (or JwtUtil.verify(token)) to perform
signature and expiry checks, extract the subject from the validated Jwt object
instead of manual string parsing, set the Principal from that subject, catch
JwtException (or the util's exception) and log only a safe non-PII message at
WARN/ERROR while keeping any claim details at DEBUG or removing them entirely,
and ensure a JwtDecoder bean (e.g., Nimbus with JWK Set URI) is configured in
the application context.
| log.info( | ||
| "✅ WebSocket 인증 성공 (JWT) - User: {}", | ||
| userEmail); |
There was a problem hiding this comment.
사용자 이메일을 INFO로 로그 — DEBUG 이하로 제한하고 값 출력은 지양
인증 성공 시 사용자 이메일을 INFO로 기록하는 것은 PII 노출 위험이 큽니다. 메시지를 축약하고 DEBUG로 내리세요.
- log.info(
- "✅ WebSocket 인증 성공 (JWT) - User: {}",
- userEmail);
+ if (log.isDebugEnabled()) {
+ log.debug("WebSocket 인증 성공 (JWT) - sub 존재");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| log.info( | |
| "✅ WebSocket 인증 성공 (JWT) - User: {}", | |
| userEmail); | |
| if (log.isDebugEnabled()) { | |
| log.debug("WebSocket 인증 성공 (JWT) - sub 존재"); | |
| } |
🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 108-108: Insertion of sensitive information into log files
This potentially sensitive information is written to a log file.
🤖 Prompt for AI Agents
In src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java around
lines 106-108, the code logs the authenticated user's email at INFO level which
exposes PII; change the log to a lower level and remove the userEmail value from
the message. Replace the current log.info(...) call with a log.debug(...) call
and remove the "{}" placeholder and userEmail argument so the message is a short
non-PII string like "✅ WebSocket 인증 성공 (JWT)". Ensure imports/logging
configuration support debug level logging.
| log.info( | ||
| "🔐 최종 Principal 상태: {}", | ||
| accessor.getUser() != null | ||
| ? accessor.getUser().getName() | ||
| : "null"); |
There was a problem hiding this comment.
최종 Principal 이름 INFO 로그 — PII 노출, 존재 여부만 DEBUG로 기록
사용자 식별자를 INFO로 쓰지 마세요. 존재 여부만 DEBUG로 남기는 것이 안전합니다.
- log.info(
- "🔐 최종 Principal 상태: {}",
- accessor.getUser() != null
- ? accessor.getUser().getName()
- : "null");
+ if (log.isDebugEnabled()) {
+ log.debug("🔐 최종 Principal 설정 여부: {}", accessor.getUser() != null);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| log.info( | |
| "🔐 최종 Principal 상태: {}", | |
| accessor.getUser() != null | |
| ? accessor.getUser().getName() | |
| : "null"); | |
| if (log.isDebugEnabled()) { | |
| log.debug("🔐 최종 Principal 설정 여부: {}", accessor.getUser() != null); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/org/scoula/global/websocket/config/WebSocketConfig.java around
lines 125 to 129, the INFO log prints the Principal's name (PII); change it so
INFO no longer contains any user identifier or presence details (e.g., use a
generic message like "최종 Principal 상태 확인 완료"), and move the existence check to a
DEBUG log that records only whether accessor.getUser() is null or not (no name
or identifier). Ensure no PII is logged at INFO level and that the DEBUG message
is the only place that records the presence boolean.
| violation == null | ||
| ? "null" | ||
| : violation.toString().replaceAll("[\\r\\n]", " "); | ||
| log.info("위반 사항 {}: {}", i + 1, sanitizedViolation); |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the log injection vulnerability, we should ensure that all user-controlled fields of the Violation object are sanitized before being logged. Instead of relying on violation.toString(), we should explicitly extract each field, sanitize it (removing newlines, carriage returns, and other control characters), and then log the sanitized values. This prevents any malicious input from forging log entries or injecting unwanted content. The fix should be applied in src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java, specifically in the block where violations are logged (around line 2521).
| @@ -2514,11 +2514,20 @@ | ||
|
|
||
| for (int i = 0; i < violations.size(); i++) { | ||
| LegalityDTO.Violation violation = violations.get(i); | ||
| String sanitizedViolation = | ||
| violation == null | ||
| ? "null" | ||
| : violation.toString().replaceAll("[\\r\\n]", " "); | ||
| log.info("위반 사항 {}: {}", i + 1, sanitizedViolation); | ||
| String sanitizedLawName = violation.getLawName() == null ? "정보 없음" : violation.getLawName().replaceAll("[\\r\\n]", " "); | ||
| String sanitizedViolationContent = violation.getViolationContent() == null ? "정보 없음" : violation.getViolationContent().replaceAll("[\\r\\n]", " "); | ||
| String sanitizedExplanation = violation.getExplanation() == null ? "정보 없음" : violation.getExplanation().replaceAll("[\\r\\n]", " "); | ||
| String sanitizedOriginalClause = violation.getOriginalClause() == null ? "정보 없음" : violation.getOriginalClause().replaceAll("[\\r\\n]", " "); | ||
| String sanitizedLegalBasis = violation.getLegalBasis() == null ? "정보 없음" : violation.getLegalBasis().replaceAll("[\\r\\n]", " "); | ||
| String sanitizedImprovementExample = violation.getImprovementExample() == null ? "정보 없음" : violation.getImprovementExample().replaceAll("[\\r\\n]", " "); | ||
| log.info("위반 사항 {}: 법령='{}', 내용='{}', 설명='{}', 원조항='{}', 법적근거='{}', 개선방안='{}'", | ||
| i + 1, | ||
| sanitizedLawName, | ||
| sanitizedViolationContent, | ||
| sanitizedExplanation, | ||
| sanitizedOriginalClause, | ||
| sanitizedLegalBasis, | ||
| sanitizedImprovementExample); | ||
| StringBuilder violationMessage = new StringBuilder(); | ||
|
|
||
| violationMessage.append( |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (3)
2708-2719: 최종 특약 삭제 수락 키 불일치로 인한 기능 오동작
requestFinalContractDeletion에서 Redis 키는final-contract:deletion:{contractChatId}:{ownerId}로 저장(값=clauseOrder)하지만,acceptFinalContractDeletion에서는final-contract:deletion:{contractChatId}:{clauseOrder}로 조회(값=ownerId 기대)하고 있어 키/값 정의가 뒤바뀌었습니다. 현재 코드는 항상 키 미발견/검증 실패 가능성이 큽니다. 아래와 같이 키/값 해석을 요청 측과 일치시키세요.- String redisKey = "final-contract:deletion:" + contractChatId + ":" + clauseOrder; - String storedOwnerId = stringRedisTemplate.opsForValue().get(redisKey); - - if (storedOwnerId == null) { + String redisKey = "final-contract:deletion:" + contractChatId + ":" + contractChat.getOwnerId(); + String storedClauseOrderStr = stringRedisTemplate.opsForValue().get(redisKey); + + if (storedClauseOrderStr == null) { throw new BusinessException( ChatErrorCode.CONTRACT_END_REQUEST_NOT_FOUND, "삭제 요청이 존재하지 않습니다."); } - - if (!storedOwnerId.equals(contractChat.getOwnerId().toString())) { - throw new BusinessException( - ChatErrorCode.CONTRACT_END_REQUEST_INVALID, "삭제 요청 정보가 유효하지 않습니다."); - } + Integer storedClauseOrder = Integer.parseInt(storedClauseOrderStr); + if (!Objects.equals(storedClauseOrder, clauseOrder)) { + throw new BusinessException( + ChatErrorCode.CONTRACT_END_REQUEST_INVALID, "삭제 요청 정보가 유효하지 않습니다."); + }
2394-2397: 수정 요청 대기 여부 키 불일치(오탑재)로 항상 false/오검 가능수정 요청 저장 키는
"final-contract:modification:{contractChatId}:{ownerId}"(값에 clauseOrder 포함)인데, 여기서는"final-contract:modification:{contractChatId}:{clauseOrder}"로 조회하고 있습니다. 키 스킴을 맞추고, 저장된 JSON의 clauseOrder를 비교해야 합니다.- public boolean hasPendingModificationRequest(Long contractChatId, Integer clauseOrder) { - String redisKey = "final-contract:modification:" + contractChatId + ":" + clauseOrder; - return stringRedisTemplate.hasKey(redisKey); - } + public boolean hasPendingModificationRequest(Long contractChatId, Integer clauseOrder) { + ContractChat contractChat = contractChatMapper.findByContractChatId(contractChatId); + if (contractChat == null) return false; + String redisKey = "final-contract:modification:" + contractChatId + ":" + contractChat.getOwnerId(); + String valueDataJson = stringRedisTemplate.opsForValue().get(redisKey); + if (valueDataJson == null) return false; + try { + com.fasterxml.jackson.databind.JsonNode rootNode = objectMapper.readTree(valueDataJson); + Integer storedClauseOrder = rootNode.get("clauseOrder").asInt(); + return Objects.equals(storedClauseOrder, clauseOrder); + } catch (Exception e) { + log.warn("수정 요청 키 파싱 실패: {}", e.getMessage()); + return false; + } + }
128-150: Spring STOMP 사용자 대상 큐 메시지 미전달 위험: Principal.name()과 convertAndSendToUser 인자 불일치 확인 필요현재 설정
- WebSocketConfig.java (
configureMessageBroker)
• setUserDestinationPrefix("/user")
• Principal.name() = JWT ‘sub’(이메일)- ContractChatServiceImpl.java (lines 122–123) / ContractChatControllerImpl.java (lines 135–136)
• messagingTemplate.convertAndSendToUser(dto.getSenderId().toString(), "/queue/contract/error", …)문제
convertAndSendToUser()가 내부적으로/user/{principal.name}{destination}으로 라우팅되므로, Principal.getName()과 전달한 사용자 식별자(여기서는 senderId)가 일치하지 않으면 메시지가 소비자에게 도달하지 않습니다.해결 방안 (택1)
- WebSocketConfig에서 UsernamePasswordAuthenticationToken 구성 시
userEmail대신 사용자 ID를 Principal.name()으로 설정- Service/Controller에서 convertAndSendToUser 호출 인자를
principal.getName()으로 변경예시 수정안
- // OFFLINE_USER 에러 전송 - messagingTemplate.convertAndSendToUser( - dto.getSenderId().toString(), "/queue/contract/error", errorInfo); + // OFFLINE_USER 에러 전송 (Principal.name() 사용) + messagingTemplate.convertAndSendToUser( + principal.getName(), "/queue/contract/error", errorInfo);
- 적용 위치
• src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java:122–123
• src/main/java/org/scoula/domain/chat/controller/ContractChatControllerImpl.java:135–136
• (필요 시) WebSocketConfig 채널 인터셉터 또는 JWT 파싱 로직
♻️ Duplicate comments (1)
src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java (1)
37-46: Authentication NPE 방지 로직 누락 (이전 코멘트와 동일 이슈 재발)
authentication혹은authentication.getName()이 null/blank이면 NPE가 발생합니다. 방어 로직을 추가해 주세요. 아래와 같이 간단히 보완 가능합니다.- private Long getUserIdFromAuthentication(Authentication authentication) { - String currentUserEmail = authentication.getName(); - Optional<User> currentUserOpt = userService.findByEmail(currentUserEmail); + private Long getUserIdFromAuthentication(Authentication authentication) { + if (authentication == null || authentication.getName() == null || authentication.getName().isBlank()) { + throw new BusinessException(ChatErrorCode.USER_NOT_FOUND); + } + String currentUserEmail = authentication.getName(); + Optional<User> currentUserOpt = userService.findByEmail(currentUserEmail); if (currentUserOpt.isEmpty()) { throw new BusinessException(ChatErrorCode.USER_NOT_FOUND); } return currentUserOpt.get().getUserId(); }
🧹 Nitpick comments (6)
src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java (2)
213-230: Swagger 문서화 누락 및 에러 처리 일관성 불일치
- 가이드라인: 모든 컨트롤러 엔드포인트에 Swagger 애노테이션 추가 필요.
- 본 클래스 내 다른 메서드는 전역 예외 처리에 위임하는 패턴인데, 본 두 엔드포인트만 try/catch로 로컬 처리하여 응답 형식/HTTP Status가 불일치합니다. 전역 예외 처리로 정렬하거나, 전체 컨트롤러에서 일관된 패턴을 사용해 주세요.
예시(요지):
+ @Operation(summary = "최종 특약 확정 요청", description = "임대인이 임차인에게 최종 특약 확정 요청을 전송합니다.") + @ApiResponses({ + @ApiResponse(responseCode = "200", description = "요청 전송 성공"), + @ApiResponse(responseCode = "400", description = "비즈니스 예외"), + @ApiResponse(responseCode = "500", description = "서버 오류") + }) @PostMapping("/specialContract/final-request") public ResponseEntity<ApiResponse<String>> requestFinalContract(...) + @Operation(summary = "최종 특약 확정 수락/거절", description = "임차인이 최종 특약 확정 요청을 수락 또는 거절합니다.") @PostMapping("/specialContract/final-accept") public ResponseEntity<ApiResponse<Map<String, Object>>> acceptFinalContract(...)또한, 에러 처리는 전역 예외 처리기(@ControllerAdvice)로 위임하여 응답 포맷/상태 코드를 일관화할 것을 권장합니다. 원하시면 전체 클래스의 예외 처리 정렬 PR 패치를 도와드릴게요.
Also applies to: 232-254
213-216: 사용자 식별 방식의 불일치: Authentication vs @AuthenticationPrincipal본 클래스의 대부분은
@AuthenticationPrincipal CustomUserDetails로userId를 직접 사용하지만, 신규 엔드포인트 2곳만Authentication→ 이메일 조회 → DB 조회로userId를 간접 획득합니다. 성능/일관성을 위해 동일하게@AuthenticationPrincipal을 사용하거나, 공통 헬퍼를 통일하는 것을 권장합니다.Also applies to: 232-237
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (4)
2496-2498: 필드 ObjectMapper가 있는데 지역 ObjectMapper 재생성이미 필드로
private final ObjectMapper objectMapper = new ObjectMapper();가 존재합니다. 지역 변수 재생성을 제거하고 필드를 재사용해 주세요.- ObjectMapper objectMapper = new ObjectMapper(); - sanitizedLegalityResponse = objectMapper.writeValueAsString(legalityResponse); + // 재사용: 클래스 필드 objectMapper + sanitizedLegalityResponse = objectMapper.writeValueAsString(legalityResponse);
497-513: 온라인 상태 관리 이중 소스(메모리 vs Redis)로 혼동 발생현재 온라인 상태는 Redis 기반(
roomKey)으로 전환되었는데, 메모리 맵(contractChatOnlineUsers) 및setContractChatUserOnline/offline은 일부 경로에서만 사용되어 상태 소스가 이중화되었습니다. 유지 보수를 위해 한 소스로 통일하거나, 메모리 기반 코드를 제거하는 것을 권장합니다.Also applies to: 627-652, 655-665
574-590: 로그 주입 방지 및 로그 노이즈 완화여러 곳에서 사용자 입력(제목/내용 등)을 그대로 로그에 출력합니다. CR/LF 제거 및 길이 제한으로 로그 주입/포이즈닝 위험을 줄이고, INFO → DEBUG 레벨로 낮추는 것도 고려하세요.
예시 유틸:
private String sanitizeForLog(String s) { if (s == null) return "null"; String cleaned = s.replaceAll("[\\r\\n]", " "); return cleaned.length() > 300 ? cleaned.substring(0, 300) + "..." : cleaned; }사용 예:
log.debug("title: '{}'", sanitizeForLog(title)); log.debug("content: '{}'", sanitizeForLog(content));Also applies to: 1374-1377, 1461-1466, 2032-2057
152-167: AI 발신자 ID 매직 넘버 정리
9999L, 9998L, 9997L, 9996L등의 매직 넘버를 상수/enum으로 추출해 가독성과 변경 용이성을 높이세요.예:
private static final long SENDER_AI = 9999L; private static final long SENDER_AI_NEXT = 9997L; private static final long SENDER_AI_BTN = 9998L; private static final long SENDER_AI_LEGAL = 9996L;Also applies to: 169-185, 186-202, 203-219
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java(9 hunks)src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java(8 hunks)src/main/java/org/scoula/global/config/ServletConfig.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/scoula/global/config/ServletConfig.java
🧰 Additional context used
📓 Path-based instructions (3)
src/main/java/org/scoula/**/service/*Service{Interface,Impl}.java
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use interface-implementation pattern for services:
*ServiceInterface.java(interface) and*ServiceImpl.java(implementation)
Files:
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java
src/main/java/org/scoula/**/controller/*Controller{,Impl}.java
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use interface-implementation pattern for controllers:
*Controller.java(interface) and*ControllerImpl.java(implementation)
Files:
src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java
src/main/java/org/scoula/**/controller/**/*.java
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/main/java/org/scoula/**/controller/**/*.java: All controller endpoints should return a consistentApiResponse<T>wrapper
Add Swagger annotations to controller endpoints for API documentation
Files:
src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java
🧬 Code Graph Analysis (2)
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (2)
src/main/java/org/scoula/global/common/exception/BusinessException.java (1)
BusinessException(4-21)src/main/java/org/scoula/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(4-13)
src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java (1)
src/main/java/org/scoula/global/common/exception/BusinessException.java (1)
BusinessException(4-21)
🪛 GitHub Check: CodeQL
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java
[failure] 2502-2502: Log Injection
This log entry depends on a user-provided value.
[failure] 2521-2521: Log Injection
This log entry depends on a user-provided value.
[failure] 2568-2568: Log Injection
This log entry depends on a user-provided value.
🔇 Additional comments (4)
src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java (2)
98-127: 가격 관련 엔드포인트 라우팅 변경 적합
/price/request(POST),/price/reject(POST),/price/accept(PATCH)로의 라우팅 정리가 의미상 명확해졌습니다.- 서비스 위임 시그니처도 일관적입니다.
136-148: 적법성/특약 저장 로직의 서비스 분리 적절
contractFixService로 역할 분리되어 관심사의 분리가 개선되었습니다.- 컨트롤러는 DTO 입출력/보일러플레이트에 집중하고 서비스가 도메인 로직을 담당하는 구조로 깔끔합니다.
src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java (2)
2813-2831: Presence 브로드캐스트 로직 적절Redis 기반 멤버십을 반영하여 실시간 Presence 페이로드를 topic으로 방송하는 접근이 명확하고, 클라이언트 동기화에 유용합니다.
203-219: 적법성 전용 AI 메시지 채널 추가 적절법률 관련 메시지를 별도 발신자 코드로 분리해 UX 및 필터링에 유리합니다.
| // [적법성 검사] 계약서 1 몽고DB에 특약 저장 | ||
| contractFixService.saveSpecialContract(contractChatId, buyerId); | ||
| try { | ||
| Thread.sleep(2000); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| } | ||
| AiMessageNext(contractChatId, "다음은 마지막 4단계: '적법성 검토' 단계입니다."); | ||
| try { | ||
| Thread.sleep(2000); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| } | ||
| AiMessage(contractChatId, "AI가 지금까지 작성된 계약서의 적법성을 분석중이에요!\n 잠시만 기다려주세요!"); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
서비스 계층에서 Thread.sleep 사용으로 인한 성능/확장성 저하
acceptFinalContractConfirmation에서 다수의 Thread.sleep(2000) 호출이 있습니다. 이는 요청 처리 스레드를 불필요하게 블로킹하고, 트랜잭션 경계(@transactional) 내에서 지연을 유발할 수 있어 TPS를 크게 떨어뜨립니다. 메시지 딜레이는 비동기/스케줄링으로 대체하세요.
권장:
- 비즈니스 트랜잭션은 빠르게 종료하고, 딜레이가 필요한 STOMP 전송/AI 메시지는
- @async 메서드,
- TaskScheduler.schedule 또는 ScheduledExecutorService,
- 이벤트 발행(ApplicationEventPublisher) + 비동기 리스너
로 처리. 필요 시 메시지 배치를 통해 프론트에서 순차 표기.
원하시면 해당 메서드를 이벤트 기반/비동기로 리팩토링한 패치를 제공하겠습니다.
Also applies to: 2491-2579
🚀 관련 이슈
#37
🔑 주요 변경사항
✔️ 체크 리스트
mainbranch에 실수로 PR 생성 금지)📢 To Reviewers
📸 스크린샷 or 실행영상
Summary by CodeRabbit
신기능
변경